-
Notifications
You must be signed in to change notification settings - Fork 21
Adding cosi specific changes to prow.sh #9
Adding cosi specific changes to prow.sh #9
Conversation
/assign @xing-yang |
release-tools/prow.sh
Outdated
configvar CSI_PROW_E2E_FOCUS_1_16 '^' "non-alpha, feature-tagged tests for Kubernetes = 1.16" # no tests to run, match nothing | ||
configvar CSI_PROW_E2E_FOCUS_LATEST '\[Feature:VolumeSnapshotDataSource\]' "non-alpha, feature-tagged tests for Kubernetes >= 1.17" | ||
configvar CSI_PROW_E2E_FOCUS "$(get_versioned_variable CSI_PROW_E2E_FOCUS "${csi_prow_kubernetes_version_suffix}")" "non-alpha, feature-tagged tests" | ||
configvar COSI_PROW_E2E_FOCUS "$(get_versioned_variable COSI_PROW_E2E_FOCUS "${cosi_prow_kubernetes_version_suffix}")" "non-alpha, feature-tagged tests" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if the tag is relevant for COSI. For CSI tests, tests tagged with feature gates are only run in clusters that support them. We can keep them for now and remove them if we are sure they are not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
release-tools/prow.sh
Outdated
# Installs all nessesary CRDs | ||
install_crds() { | ||
# Wait until cosi CRDs are in place. | ||
# CRD_BASE_DIR="https://github.com/kubernetes-sigs/container-object-storage-interface-api/crds" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
release-tools/prow.sh
Outdated
install_crds() { | ||
# Wait until cosi CRDs are in place. | ||
# CRD_BASE_DIR="https://github.com/kubernetes-sigs/container-object-storage-interface-api/crds" | ||
CRD_BASE_DIR="https://raw.githubusercontent.com/brahmaroutu/api/proper_crds/crds" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be from an official repo?
If you are still testing, can you mark your PR as "WIP"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed it to use the official site (now available). But I need to work with a different error,
"Required value: protected groups must have approval annotation "api-approved.kubernetes.io", see kubernetes/enhancements#1111"
I am implementing a workaround using sed to insert the annotation until we move these api group into k8s build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use this PR "kubernetes-retired/container-object-storage-interface-api#2" for now and replace it with the approved PR in the future when it is officially approved by the API reviewer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
release-tools/prow.sh
Outdated
# Wait until cosi CRDs are in place. | ||
# CRD_BASE_DIR="https://github.com/kubernetes-sigs/container-object-storage-interface-api/crds" | ||
CRD_BASE_DIR="https://raw.githubusercontent.com/brahmaroutu/api/proper_crds/crds" | ||
kubectl apply -f "${CRD_BASE_DIR}/objectstorage.k8s.io_bucketclasses.yaml" --validate=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The install location should be dynamic. If this is testing a PR at the API repo "https://github.com/kubernetes-sigs/container-object-storage-interface-api", it should read from the API repo. In other cases, it should either install from master or a stable branch of the API repo.
See this PR for example: kubernetes-csi/csi-release-tools#113
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
release-tools/prow.sh
Outdated
# Install controller and associated RBAC, retrying until the pod is running. | ||
install_controller() { | ||
#kubectl apply -f "https://raw.githubusercontent.com/kubernetes-sigs/container-object-storage-interface-controller/${COSI_CONTROLLER_VERSION}/deploy/controller.yaml" | ||
kubectl apply -f "https://raw.githubusercontent.com/container-object-storage-interface/cosi-controller-manager/master/samples/rbac.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments earlier about install location should be dynamic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
release-tools/prow.sh
Outdated
done | ||
|
||
|
||
kubectl apply -f "https://raw.githubusercontent.com/kubernetes-csi/external-snapshotter/${CSI_SNAPSHOTTER_VERSION}/deploy/kubernetes/snapshot-controller/setup-snapshot-controller.yaml" | ||
#kubectl apply -f "https://raw.githubusercontent.com/kubernetes-sigs/container-object-storage-interface-controller/${COSI_CONTROLLER_VERSION}/deploy/rbac.yaml" | ||
kubectl apply -f "https://raw.githubusercontent.com/brahmaroutu/cosi-controller-manager/test/samples/controller.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comments earlier about install location should be dynamic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
release-tools/prow.sh
Outdated
done | ||
#docker exec cosi-prow-control-plane docker image ls --format='{{.Repository}} {{.Tag}} {{.ID}}' | grep -e COSI -e hostpath | while read -r repo tag id; do | ||
# echo "$repo" "$tag" "$(docker exec cosi-prow-control-plane docker image inspect --format='{{ index .Config.Labels "revision"}}' "$id")" | ||
#done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove if no longer needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
release-tools/prow.sh
Outdated
fi | ||
echo "Srini Collect Cluster Info" | ||
collect_cluster_info | ||
echo "Srini Done Collect Cluster Info" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like these are your temporary debug messages.
Can you mark this PR as WIP? Looks like you are still testing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
d6ba67a
to
6457831
Compare
release-tools/prow.sh
Outdated
CRD_BASE_DIR="https://raw.githubusercontent.com/kubernetes-sigs/container-object-storage-interface-api/master/crds" | ||
kubectl_apply "${CRD_BASE_DIR}/objectstorage.k8s.io_bucketclasses.yaml" | ||
kubectl_apply "${CRD_BASE_DIR}/objectstorage.k8s.io_bucketrequests.yaml" | ||
kubectl_apply "${CRD_BASE_DIR}/objectstorage.k8s.io_buckets.yaml" | ||
kubectl_apply "${CRD_BASE_DIR}/objectstorage.k8s.io_bucketaccessclasses.yaml" | ||
kubectl_apply "${CRD_BASE_DIR}/objectstorage.k8s.io_bucketaccessrequests.yaml" | ||
kubectl_apply "${CRD_BASE_DIR}/objectstorage.k8s.io_bucketaccesses.yaml" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since kubernetes-retired/container-object-storage-interface-api#13 there's a kustomization.yaml
in https://github.com/kubernetes-sigs/container-object-storage-interface-api/tree/master/crds wich allows to deploy the CRDs (including required labels
) without exhaustively listing all of them here.
You could use, as an example, kustomize build "https://github.com/kubernetes-sigs/container-object-storage-interface-api/crds/?ref=master" | kubectl apply -f -
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would make this effort in the subsequent PRs as the 'kustomize' require additional effort, kind clusters may not bu default have Kustomize.
6457831
to
e4385e4
Compare
@xing-yang please review, I have addressed your review comments. This runs fine but may require further changes as we introduce post submit jobs, e2e/integration testing and other cost components into the build |
cmds="$(grep '^\s*CMDS\s*=' Makefile | sed -e 's/\s*CMDS\s*=//')" | ||
# Get the image that was just built (if any) from the | ||
# top-level Makefile CMDS variable and set the | ||
# deploy.sh env variables for it. We also need to | ||
# side-load those images into the cluster. | ||
for i in $cmds; do | ||
e=$(echo "$i" | tr '[:lower:]' '[:upper:]' | tr - _) | ||
images="$images ${e}_REGISTRY=none ${e}_TAG=csiprow" | ||
images="$images ${e}_REGISTRY=quay.io/containerobjectstorage ${e}_TAG=cosiprow" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you going to use quay.io to store images, not k8s.gcr.io?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed at the review meeting. Will use quay.io for now and move to k8s.gcr.io in the future.
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brahmaroutu, xing-yang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
prow.sh is modifies to use COSI specific config variables. It installs crds, controller and has hooks to run serial and parallel e2e tests.