Skip to content

Fix unbound variable used by Prow #8697

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.

Sign up for GitHub

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

Jump to bottom
Merged
estesp merged 1 commit into from
Jun 16, 2023
Merged

Fix unbound variable used by Prow #8697

estesp merged 1 commit into from
Jun 16, 2023

Conversation

@upodroid
Copy link
Contributor

@upodroid upodroid commented Jun 15, 2023 •
edited
Loading

/cc @dims

The change in kubernetes/test-infra#29824 failed because of an unbound variable.

Now, this change requires some changes in the jobs that grab containerd binaries from GCS.

 REDACTED  C02CW0CDP3YY  ~  Desktop  Git  test-infra   kubetest2  1⚑  $  grep -r containerd-staging .
./jobs/e2e_node/containerd/containerd-release-1.7/env:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging/containerd/release-1.7'
./jobs/e2e_node/containerd/containerd-release-1.6/env:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging/containerd/release-1.6'
./jobs/e2e_node/containerd/containerd-release-1.6-presubmit/env:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging'
./jobs/e2e_node/containerd/containerd-main-presubmit/env:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging'
./jobs/e2e_node/containerd/containerd-release-1.7-presubmit/env:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging'
./jobs/e2e_node/containerd/containerd-main/systemd/env:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging/containerd/master'
./jobs/e2e_node/containerd/containerd-main/cgroupv1/env-cgroupv1:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging/containerd/master'
./jobs/e2e_node/containerd/containerd-main/env:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging/containerd/master'
./jobs/e2e_node/containerd/containerd-main/cgroupv2/env-cgroupv2:CONTAINERD_DEPLOY_PATH: 'cri-containerd-staging/containerd/master'

I could add the variable back but I'm not sure it is really needed and containerd did change its branch from master to main.

Old GCS Path: gs://cri-containerd-staging/containerd/master/*
New GCS Path: gs://cri-containerd-staging/containerd/*

@k8s-ci-robot k8s-ci-robot requested a review from dims June 15, 2023 20:13
@k8s-ci-robot
Copy link

k8s-ci-robot commented Jun 15, 2023

Hi @upodroid. Thanks for your PR.

I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

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.

dims
dims approved these changes Jun 15, 2023
View reviewed changes
Copy link
Member

@dims dims left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dims
Copy link
Member

dims commented Jun 15, 2023

cc @mikebrow

@upodroid
fix unbound variable
d358f16 
Signed-off-by: upodroid <[email protected]>
@upodroid upodroid force-pushed the fix-unbound-variable branch from 34eb4d3 to d358f16 Compare June 15, 2023 20:18
@upodroid
Copy link
Contributor Author

upodroid commented Jun 15, 2023

On 2nd thoughts, I do need to set DEPLOY_DIR in those jobs. The release markers will be clobbered if the folders are shared between different branches.

Lets merge this anyway.

 REDACTED  C02CW0CDP3YY  ~  Desktop  Git  containerd   fix-unbound-variable  $  gsutil ls gs://cri-containerd-staging/containerd/release-1.7 | grep latest
gs://cri-containerd-staging/containerd/release-1.7/latest
 REDACTED  C02CW0CDP3YY  ~  Desktop  Git  containerd   fix-unbound-variable  $  gsutil ls gs://cri-containerd-staging/containerd/master | grep latest
gs://cri-containerd-staging/containerd/master/latest

@estesp estesp merged commit b8bd27a into containerd:main Jun 16, 2023
kiashok added a commit to kiashok/containerd that referenced this pull request Oct 23, 2024
@kiashok
Merged PR 8369437: Update fork-external/main with upstream containerd…
6b2c041 
…/main

Merge upstream containerd/main at commit 5d1ab01 into ado fork-external/main

Related work items: containerd#7944, containerd#8174, containerd#8334, containerd#8362, containerd#8572, containerd#8582, containerd#8588, containerd#8605, containerd#8606, containerd#8617, containerd#8619, containerd#8626, containerd#8627, containerd#8633, containerd#8637, containerd#8641, containerd#8643, containerd#8645, containerd#8652, containerd#8667, containerd#8672, containerd#8673, containerd#8676, containerd#8680, containerd#8684, containerd#8685, containerd#8692, containerd#8696, containerd#8697, containerd#8701, containerd#8708, containerd#8717, containerd#8726, containerd#8728, containerd#8729, containerd#8731, containerd#8732, containerd#8740, containerd#8752, containerd#8758, containerd#8762, containerd#8764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dims dims dims approved these changes

@estesp estesp estesp approved these changes

@dcantah dcantah dcantah approved these changes

Assignees

No one assigned

Labels

needs-ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@upodroid @k8s-ci-robot @dims @estesp @dcantah