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

Update conformance image to use debian-base:buster-v1.9.0 #104696

Merged
merged 1 commit into from Sep 13, 2021
Merged

Update conformance image to use debian-base:buster-v1.9.0 #104696

merged 1 commit into from Sep 13, 2021

Conversation

PushkarJ
Copy link
Member

@PushkarJ PushkarJ commented Aug 31, 2021 •
edited by justaugustus

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

  • Debian base used was older missing multiple fixed CVEs

Special notes for your reviewer:

Not sure, how can these images be promoted to k8s.gcr.io, but I am assuming it will happen as part of next release build getting published. If not, happy to submit a PR to promote these images

/area conformance
/sig release testing architecture security

Update conformance image to use debian-base:buster-v1.9.0

@k8s-ci-robot k8s-ci-robot added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. area/conformance Issues or PRs related to kubernetes conformance tests size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/testing Categorizes an issue or PR as relevant to SIG Testing. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/security Categorizes an issue or PR as relevant to SIG Security. 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. area/test labels Aug 31, 2021
@PushkarJ
Copy link
Member Author

/release-note-none

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Aug 31, 2021
@PushkarJ
Copy link
Member Author

/retest-required

neolit123
neolit123 reviewed Aug 31, 2021
View reviewed changes
Copy link
Member

@neolit123 neolit123 left a comment

Choose a reason for hiding this comment

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

Not sure, how can these images be promoted to k8s.gcr.io, but I am assuming it will happen as part of next release build getting published. If not, happy to submit a PR to promote these images

the conformance image would be automatically build / published as part of the release process, but the base image should be promoted manually AFAIK.

here is an example PR:
kubernetes/k8s.io#2546

the docs are here:
https://github.com/kubernetes/k8s.io/blob/5f267ada6aeca5d271416a3490bc6ba7023b6839/k8s.gcr.io/README.md

@neolit123
Copy link
Member

/triage accepted
/priority important-soon

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. 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 Aug 31, 2021
@neolit123
Copy link
Member

/milestone v1.23

@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Aug 31, 2021
@PushkarJ
Copy link
Member Author

PushkarJ commented Sep 1, 2021

Thank you @neolit123 for the pointers.

AFAIK, the base image does not need promotion as we have been pulling distroless/static (debian 10) from similar location as the one in this PR as per this Dockerfile and Makefile

Long term it seems the idea is to use go-runner under k/release in favor of this which is under k/k as per #102490, so this PR is doing a bit of that by switching the base image location that is equivalent to go-runner in k/release :)

justaugustus
justaugustus requested changes Sep 2, 2021
View reviewed changes
Copy link
Member

@justaugustus justaugustus left a comment

Choose a reason for hiding this comment

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

Thanks for filing this, @PushkarJ!

A few changes/notes:

/hold

test/conformance/image/Makefile Outdated Show resolved Hide resolved
test/conformance/image/Makefile Outdated Show resolved Hide resolved
test/conformance/image/Makefile Outdated Show resolved Hide resolved
test/conformance/image/Makefile Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 2, 2021
@PushkarJ PushkarJ changed the title Bump conformance base images [WIP] Bump conformance base images Sep 3, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 3, 2021
@justaugustus justaugustus changed the title [WIP] Bump conformance base images Update conformance image to use debian-base:buster-v1.9.0 Sep 4, 2021
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. labels Sep 4, 2021
@justaugustus
Copy link
Member

Nice work, @PushkarJ! I've updated the description a little now that we're not using bullseye.
/lgtm
/approve
/assign @spiffxp

@justaugustus
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 4, 2021
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 4, 2021
@justaugustus
Copy link
Member

/area dependency security

@k8s-ci-robot k8s-ci-robot added area/dependency Issues or PRs related to dependency changes area/security labels Sep 4, 2021
@spiffxp
Copy link
Member

spiffxp commented Sep 8, 2021

Is there some way we could get diff without debian? Then we could reduce the number of things that need to be bumped

kubernetes/test/conformance/image/Dockerfile

Lines 22 to 23 in 5be7bb4

# This is a dependency for `kubectl diff` tests
COPY --from=debbase /usr/bin/diff /usr/local/bin/

@PushkarJ
Copy link
Member Author

PushkarJ commented Sep 9, 2021 •
edited

Is there some way we could get diff without debian? Then we could reduce the number of things that need to be bumped

kubernetes/test/conformance/image/Dockerfile

Lines 22 to 23 in 5be7bb4

# This is a dependency for `kubectl diff` tests
COPY --from=debbase /usr/bin/diff /usr/local/bin/

Only prior art I found was for etcd image, that does something similar for bash-static i.e. Copy bash-static from debian to distroless. Link: https://github.com/kubernetes/kubernetes/blob/master/cluster/images/etcd/Dockerfile#L20-L27

@PushkarJ
Bump conformance images to use debian:buster-v1.9.0
8ed3151 
- Debian base used was older (v2.1.3)  missing multiple fixed CVEs
- Minor update to distroless debian image name to explicitly point
  to debian 10
- Debian base image now points to buster-1.9.0
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 13, 2021
@dims
Copy link
Member

dims commented Sep 13, 2021

cc @johnSchnake

spiffxp
spiffxp approved these changes Sep 13, 2021
View reviewed changes
Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

/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 Sep 13, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justaugustus, PushkarJ, spiffxp

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 Sep 13, 2021
@k8s-ci-robot k8s-ci-robot merged commit caf853b into kubernetes:master Sep 13, 2021
@johnSchnake
Copy link
Contributor

Sorry for taking a while; I know it merged but fwiw I also built it and ran sonobuoy using that image without an issue. LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neolit123 neolit123 neolit123 left review comments

@justaugustus justaugustus justaugustus requested changes

@spiffxp spiffxp spiffxp approved these changes

@timothysc timothysc Awaiting requested review from timothysc

Assignees

@spiffxp spiffxp

@justaugustus justaugustus

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/conformance Issues or PRs related to kubernetes conformance tests area/dependency Issues or PRs related to dependency changes area/security area/test 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-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/release Categorizes an issue or PR as relevant to SIG Release. sig/security Categorizes an issue or PR as relevant to SIG Security. sig/testing Categorizes an issue or PR as relevant to SIG Testing. 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.23
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@PushkarJ @neolit123 @justaugustus @spiffxp @dims @k8s-ci-robot @johnSchnake