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.

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

Remove unsupported Windows SAC images from pause image #107056

Merged
merged 1 commit into from
Mar 12, 2022
Merged

Remove unsupported Windows SAC images from pause image #107056

merged 1 commit into from
Mar 12, 2022

Conversation

jsturtevant
Copy link
Contributor

@jsturtevant jsturtevant commented Dec 15, 2021 •
edited
Loading

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Removes Windows SAC releases are no longer supported by Microsoft. This removes the support in k8s pause image and reduces storage space in registry for newer versions of pause image.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Windows Pause no longer has support for SAC releases 1903, 1909, 2004. Windows image support is now Ltcs 2019 (1809), 20H2, LTSC 2022

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

/sig windows

/cc @claudiubelu @marosset @ravisantoshgudimetla

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/windows Categorizes an issue or PR as relevant to SIG Windows. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Dec 15, 2021
@jsturtevant
Copy link
Contributor Author

pod pending timed out
/retest

@claudiubelu
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 16, 2021
ravisantoshgudimetla
ravisantoshgudimetla approved these changes Dec 16, 2021
View reviewed changes
Copy link
Contributor

@ravisantoshgudimetla ravisantoshgudimetla left a comment

Choose a reason for hiding this comment

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

/lgtm

Thank you @jsturtevant

@jsturtevant
Copy link
Contributor Author

/assign @cblecker @marosset

@marosset
Copy link
Contributor

Do we need to wait for kubernetes/test-infra#24689 to merge for this PR too?

@claudiubelu
Copy link
Contributor

Looking at it again, I think we need to bump the pause image version as well, since 3.6 was already promoted. Additionally, we should add a note about these changes into the pause image Change logs: https://github.com/kubernetes/kubernetes/blob/master/build/pause/CHANGELOG.md

Do we need to wait for kubernetes/test-infra#24689 to merge for this PR too?

I don't think so. The test jobs are running using promoted images. If this gets merged, the postsubmit job will build the image and push it to the staging registry, it's not automatically promoted.

@dims
Copy link
Member

dims commented Jan 6, 2022

Yes, please bump to 3.7 https://github.com/kubernetes/kubernetes/blob/master/build/pause/Makefile#L20

@dims
Copy link
Member

dims commented Jan 6, 2022

+1 to use what @claudiubelu said as the rule of thumb, if the TAG has a version that is promoted to the public repository then we need to bump the TAG for any changes that changes the payload of the image.

@marosset
Copy link
Contributor

marosset commented Jan 6, 2022

Yes, please bump to 3.7 https://github.com/kubernetes/kubernetes/blob/master/build/pause/Makefile#L20

3.7 or 3.6.1?

@marosset
Copy link
Contributor

marosset commented Jan 6, 2022

It seems like we are not being consistent w/ pause image versioning.
Sometimes we've bumped the minor version when adding support for new flavors of Windows (3.3 -> 3.4) and other times we've just bumped the patch version (3.4 -> 3.4.1).

@jsturtevant
Copy link
Contributor Author

3.7 makes sense to me, it is a breaking change as we are dropping support not adding additional

@marosset
Copy link
Contributor

marosset commented Jan 7, 2022

3.7 makes sense to me, it is a breaking change as we are dropping support not adding additional

Works for me!

@claudiubelu
Copy link
Contributor

3.7 works for me as well. Don't forget to add a note for the 3.7 image in the pause image's Changelog as well: https://github.com/kubernetes/kubernetes/blob/master/build/pause/CHANGELOG.md

Also, in your PR message, in the Does this PR introduce a user-facing change? Section, you mention Ltcs images, but it's actually LTSC. AFAIK that section ends up in the release notes, so we should make sure they're correct.

@jsturtevant jsturtevant force-pushed the remove-sac-from-pause branch from 15e14b4 to 7a15440 Compare January 11, 2022 20:14
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 11, 2022
@jsturtevant
Copy link
Contributor Author

3.7 works for me as well. Don't forget to add a note for the 3.7 image in the pause image's Changelog as well: https://github.com/kubernetes/kubernetes/blob/master/build/pause/CHANGELOG.md

Also, in your PR message, in the Does this PR introduce a user-facing change? Section, you mention Ltcs images, but it's actually LTSC. AFAIK that section ends up in the release notes, so we should make sure they're correct.

updated, including the Release note section

@jsturtevant jsturtevant force-pushed the remove-sac-from-pause branch from 7a15440 to b6599b1 Compare January 11, 2022 20:23
@jsturtevant
Remove unsupported Windows SAC images from pause image
c5e341d 
Signed-off-by: James Sturtevant <jstur@microsoft.com>
@jsturtevant jsturtevant force-pushed the remove-sac-from-pause branch from b6599b1 to c5e341d Compare January 11, 2022 20:40
@jsturtevant
Copy link
Contributor Author

/assign @cblecker @marosset @claudiubelu

@jsturtevant
Copy link
Contributor Author

/assign @spiffxp @claudiubelu @marosset

marosset
marosset approved these changes Feb 22, 2022
View reviewed changes
Copy link
Contributor

@marosset marosset left a comment

Choose a reason for hiding this comment

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

LGTM

@marosset
Copy link
Contributor

/triage accepted
/priority important-longterm

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Mar 10, 2022
@jsturtevant
Copy link
Contributor Author

/assign @dims

@dims
Copy link
Member

dims commented Mar 11, 2022

/approve
/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, jsturtevant, marosset, ravisantoshgudimetla

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 11, 2022
@k8s-ci-robot
Copy link
Contributor

@jsturtevant: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-e2e-kind-ipv6 c5e341d link unknown /test pull-kubernetes-e2e-kind-ipv6

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.

@jsturtevant
Copy link
Contributor Author

/test pull-kubernetes-e2e-kind-ipv6

@k8s-ci-robot k8s-ci-robot merged commit b2c5bd2 into kubernetes:master Mar 12, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Mar 12, 2022
@github-actions github-actions bot mentioned this pull request Mar 22, 2022
Open
@dims dims mentioned this pull request Mar 29, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marosset marosset marosset approved these changes

@ravisantoshgudimetla ravisantoshgudimetla ravisantoshgudimetla approved these changes

@claudiubelu claudiubelu Awaiting requested review from claudiubelu

Assignees

@dims dims

@spiffxp spiffxp

@cblecker cblecker

@claudiubelu claudiubelu

@ravisantoshgudimetla ravisantoshgudimetla

@marosset marosset

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. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/windows Categorizes an issue or PR as relevant to SIG Windows. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Milestone
v1.24
Development

Successfully merging this pull request may close these issues.
8 participants
@jsturtevant @claudiubelu @marosset @dims @k8s-ci-robot @ravisantoshgudimetla @spiffxp @cblecker