-
Notifications
You must be signed in to change notification settings - Fork 421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update upstream k8s CI image store reference #1512
update upstream k8s CI image store reference #1512
Conversation
/test ls |
@CecileRobertMichon: The specified target(s) for
Use
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/test pull-cluster-api-provider-azure-conformance-with-ci-artifacts |
@@ -150,7 +150,7 @@ spec: | |||
$${GSUTIL} cp "$$CI_URL/$$CI_CONTAINER.$$CONTAINER_EXT" "$$CI_DIR/$$CI_CONTAINER.$$CONTAINER_EXT" | |||
$${SUDO} ctr -n k8s.io images import "$$CI_DIR/$$CI_CONTAINER.$$CONTAINER_EXT" || echo "* ignoring expected 'ctr images import' result" | |||
$${SUDO} ctr -n k8s.io images tag k8s.gcr.io/$$CI_CONTAINER-amd64:"$${CI_VERSION//+/_}" k8s.gcr.io/$$CI_CONTAINER:"$${CI_VERSION//+/_}" |
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.
As far as I can see you can also drop those lines as k8s.gcr.io
is used. But I'm not 100% sure, maybe worth a try to get rid of those references. (The only code occurrences of the old or new ci repo I could find was the conformance images in the e2e test framework in the cluster-api repo. Apart from that we only seem to use it here.)
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.
Okay found it: kubernetes/kubernetes@8ba5a06
So looks like kubeadm >=20 (?) uses the new repo.
So if we want this to work with kubeadm <= 1.19 from ci we would have to tag both? (not sure if we want that though)
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.
It was cherry-picked pretty far back. So the newest version which has the old ci repo is v1.17.*. So I think that's not relevant as we're (probably) not testing 1.17 CI versions anywhere anymore.
@jackfrancis: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
@jackfrancis the test failed with
Is it possible you missed a reference? |
I think it fails when it tries to download kubectl and then continues with kubeadm (1.18)
|
@CecileRobertMichon this might be a required change as well: cc @chewong |
@jackfrancis I think in the last test run the problem was only that kubeadm 1.18 was used which has afaik the old CI image repo hard-coded (kubernetes/kubernetes@8ba5a06). Not sure why the kubectl download failed though. |
not sure why that would be the case, looking |
This is the issue. The preKubeadmCommand script didn't complete because of this and it proceeded to running kubeadm join without having downloaded a newer kubeadm |
I think the problem is that the kubectl download fails because it tries to use Kubernetes "v1.22.0-beta.1.140+db183316273852". Which doesn't seem to exist: https://console.cloud.google.com/storage/browser/kubernetes-release-dev/ci;tab=objects?pageState=(%22StorageObjectListTable%22:(%22f%22:%22%255B%257B_22k_22_3A_22_22_2C_22t_22_3A10_2C_22v_22_3A_22_5C_22v1.22.0-beta.1.1_5C_22_22%257D%255D%22))&project=kubernetes-release-dev&prefix=v1.22.0-beta.1.1&forceOnObjectsSortingFiltering=false Not sure where it does get the version from. The latest files point to 117 (e.g. link) |
@@ -338,7 +338,7 @@ spec: | |||
$${GSUTIL} cp "$$CI_URL/$$CI_CONTAINER.$$CONTAINER_EXT" "$$CI_DIR/$$CI_CONTAINER.$$CONTAINER_EXT" | |||
$${SUDO} ctr -n k8s.io images import "$$CI_DIR/$$CI_CONTAINER.$$CONTAINER_EXT" || echo "* ignoring expected 'ctr images import' result" | |||
$${SUDO} ctr -n k8s.io images tag k8s.gcr.io/$$CI_CONTAINER-amd64:"$${CI_VERSION//+/_}" k8s.gcr.io/$$CI_CONTAINER:"$${CI_VERSION//+/_}" | |||
$${SUDO} ctr -n k8s.io images tag k8s.gcr.io/$$CI_CONTAINER-amd64:"$${CI_VERSION//+/_}" gcr.io/kubernetes-ci-images/$$CI_CONTAINER:"$${CI_VERSION//+/_}" | |||
$${SUDO} ctr -n k8s.io images tag k8s.gcr.io/$$CI_CONTAINER-amd64:"$${CI_VERSION//+/_}" gcr.io/k8s-staging-ci-images/$$CI_CONTAINER:"$${CI_VERSION//+/_}" |
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.
we also need to change line 324 https://github.com/kubernetes-sigs/cluster-api-provider-azure/pull/1512/files#diff-f898c4eb527239bc68896f9fa132833ff4ec72509aea67f17a072940acdf2550R324
CI_URL="gs://kubernetes-release-dev/ci-periodic/$$CI_VERSION/bin/linux/amd64"
to
CI_URL="gs://k8s-release-dev/ci-periodic/$$CI_VERSION/bin/linux/amd64"
@CecileRobertMichon Looks like: cluster-api-provider-azure/test/e2e/helpers.go Lines 536 to 564 in 36ace98
https://storage.googleapis.com/k8s-release-dev/ci/latest-1.22.txt as well (not sure which is used) |
8bed5d9
to
01b961b
Compare
/test pull-cluster-api-provider-azure-conformance-with-ci-artifacts |
@CecileRobertMichon In case it's helpful. In the cluster-api repo we're using other "latest"-files to resolve the version: https://github.com/kubernetes-sigs/cluster-api/blob/master/scripts/ci-e2e-lib.sh#L89-L93 |
Ah got it. k8s-release-dev instead of kubernetes-release-dev as you wrote above. |
Conformance is running, the cluster built successfully. Let's merge this and observe CI signal in testgrid. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CecileRobertMichon 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 |
/lgtm |
@jackfrancis once this merges, could you please also open a cherry-pick PR to release-0.4 to fix https://testgrid.k8s.io/sig-cluster-lifecycle-cluster-api-provider-azure#capz-periodic-conformance-v1alpha3-k8s-main-release-0-4 ? |
What type of PR is this?
/kind failing-test
What this PR does / why we need it:
This PR updates all capz references to upstream Kubernetes CI images to the new URI, so that we can restore test signal for upstream Kubernetes commits (pre-release commits, not officially released bits).
kubernetes/k8s.io#2318
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #1510
Special notes for your reviewer:
Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
TODOs:
Release note: