Skip to content
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

Bump COS image to latest version #17770

Merged
merged 1 commit into from
Jun 1, 2020
Merged

Conversation

bsdnet
Copy link
Contributor

@bsdnet bsdnet commented May 31, 2020

Signed-off-by: Roy Yang [email protected]

kubernetes/kubernetes#91543 detect that stable image is gone. Now it silently failed. Instead, use the LTS image which will always succeed.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 31, 2020
@k8s-ci-robot
Copy link
Contributor

Hi @bsdnet. Thanks for your PR.

I'm waiting for a kubernetes 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.

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.

@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels May 31, 2020
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 31, 2020
@bsdnet
Copy link
Contributor Author

bsdnet commented May 31, 2020

/assign @vpickard @bart0sh @MHBauer
This CL uses image and get rid of image_regex, upgrade cos-73 to cos-77(cos-stable-73 does not match any image and silently fail).
In future, we should always use LTS milestone. stable is the original channel before LTS is introduced. If there are two images, it should be the latest two LTS milestones. If it has one image, it should one of the latest two LTS. The chance for breaking using LTS is really slow because
1.) LTS is much stable
2.) LTS has a long support period(15 months), and fully supported.
3.) Stable change every 6 weeks. This requires keep changing the image.

We can convert current images from cos-stables to cos-lts, image to image_family
in the future changeset. That will avoid frequent configuration file change, but also
has its drawbacks. For this changeset, try to make it small to stablize the tests first.

@dims
Copy link
Member

dims commented May 31, 2020

/ok-to-test
/approve

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 31, 2020
@bart0sh
Copy link
Contributor

bart0sh commented Jun 1, 2020

@bsdnet Would it be better to start using image_family: cos-77-lts instead? Otherwise tests will start failing every time gcp updates images that we use.

@bsdnet
Copy link
Contributor Author

bsdnet commented Jun 1, 2020

@bsdnet Would it be better to start using image_family: cos-77-lts instead?
I would suggest this for now until @MHBauer 's patch gets verified slowly. There is a drawback if
we use image_family. COS will release regularly, what should we do if the image is bad though it is
rare? We can't even rollback! We need to think through about the situation.

Otherwise tests will start failing every time gcp updates images that we use.

tests should not fail at all whether GCP publish new image or not. GCP never deleted images.Once the image was published, it will only be marked as DEPRECATED, and always be there. The reason cos-stable-73-11647-510-0 failed is that the changeset 31b01ca is wrong. It uses cos-stable-73 instead of co-73. 11647-510-0 is never released via the cos-stable channel, and so it does not exist at all. It is released in cos-73-lts family.

In short, it is very safe to use latest LTS image. As to whether to use 'image_family: cos-77-lts', let's do step by step. It also need more thinking.

@bsdnet
Copy link
Contributor Author

bsdnet commented Jun 1, 2020

/assign @derekwaynecar @dchen1107 @spiffxp

@k8s-ci-robot
Copy link
Contributor

@bsdnet: GitHub didn't allow me to assign the following users: derekwaynecar.

Note that only kubernetes members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @derekwaynecar @dchen1107 @spiffxp

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.

Copy link
Contributor

@MHBauer MHBauer left a comment

Choose a reason for hiding this comment

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

This might be the fastest way out of the hole of relying on missing images, and then we turn on the failure of non-matching regexes, and then we schedule the image updates with the bumper if necessary.

If we affirmatively pick an image it's more explicit, and does not silently fail.
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 1, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsdnet, dims, MHBauer

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. sig/testing Categorizes an issue or PR as relevant to SIG Testing. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

7 participants