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

Use debian-base instead of distroless for conformance image #119422

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Use debian-base instead of distroless for conformance image #119422

merged 1 commit into from
Jul 20, 2023

Conversation

saschagrunert
Copy link
Member

What type of PR is this?

/kind failing-test

What this PR does / why we need it:

The diff binary (required by the kubectl diff e2e test) gets statically or dynamically linked based on the used glibc version. We cannot really predict that behavior for the various platforms of debian-base and therefore cannot copy the binary around. This means that distroless is not a great choice for the conformance image unless we stop relying on diff.

This means we now switch back to debian-base for the conformance image to simplify the build process and reduce the amount of moving parts.

Which issue(s) this PR fixes:

Fixes #119411

Special notes for your reviewer:

None

Does this PR introduce a user-facing change?

Switched back to debian-base instead of distroless for conformance image.

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

None

@saschagrunert
Use debian-base instead of distroless for conformance image
1000ed6 
The `diff` binary (required by the `kubectl diff` e2e test) gets
statically or dynamically linked based on the used glibc version. We
cannot really predict that behavior for the various platforms of
debian-base and therefore cannot copy the binary around. This means that
distroless is not a great choice for the conformance image unless we
stop relying on `diff`.

This means we now switch back to `debian-base` for the conformance image
to simplify the build process and reduce the amount of moving parts.

Signed-off-by: Sascha Grunert <[email protected]>
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jul 19, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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 the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jul 19, 2023
@saschagrunert
Copy link
Member Author

cc @kubernetes/release-engineering

@k8s-ci-robot k8s-ci-robot added area/conformance Issues or PRs related to kubernetes conformance tests area/test 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/testing Categorizes an issue or PR as relevant to SIG Testing. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jul 19, 2023
@ameukam
Copy link
Member

ameukam commented Jul 19, 2023

/lgtm

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

LGTM label has been added.

Git tree hash: d6f78a0b24d8b9861fd4c535f84cd1d40fb3aee0

neolit123
neolit123 reviewed Jul 19, 2023
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.

since the bot requested review from me, i will add my 2c. switching away from distroless might need a better argument. may be better to drop the test from conformance until this is resolved?

also i don't have full context on why kubectl diff is using shell to diff, but the diff functionality can be implemented with simple standard compatible go libraries, IIUC.

@saschagrunert
Copy link
Member Author

saschagrunert commented Jul 19, 2023 •
edited

since the bot requested review from me, i will add my 2c. switching away from distroless might need a better argument. may be better to drop the test from conformance until this is resolved?

Sounds like a valid alternative to me, it's just a single test requiring this.

See #119426

also i don't have full context on why kubectl diff is using shell to diff, but the diff functionality can be implemented with simple standard compatible go libraries, IIUC.

I was thinking about this one, too. I guess this is the most appropriate mid-term solution.

@saschagrunert saschagrunert mentioned this pull request Jul 19, 2023
Closed
@saschagrunert saschagrunert changed the title WIP: Use debian-base instead of distroless for conformance image Use debian-base instead of distroless for conformance image Jul 19, 2023
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 19, 2023
@saschagrunert
Copy link
Member Author

/hold

for discussion

@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 Jul 19, 2023
saschagrunert added a commit to saschagrunert/kubernetes that referenced this pull request Jul 19, 2023
@saschagrunert
Skip kubectl diff test if binary is not found in $PATH
bf33954 
As an alternative to
kubernetes#119422, we now skip the
conformance test and therefore do not rely on `diff` any more.

Signed-off-by: Sascha Grunert <[email protected]>
@saschagrunert
Copy link
Member Author

/unhold

@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 Jul 20, 2023
@saschagrunert saschagrunert added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Jul 20, 2023
@ameukam
Copy link
Member

ameukam commented Jul 20, 2023

/milestone v1.28

@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone Jul 20, 2023
@ameukam
Copy link
Member

ameukam commented Jul 20, 2023

/assign @dims

@dims
Copy link
Member

dims commented Jul 20, 2023

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, saschagrunert

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 Jul 20, 2023
@k8s-ci-robot k8s-ci-robot merged commit 4b6c340 into kubernetes:master Jul 20, 2023
14 checks passed
@saschagrunert saschagrunert deleted the conformance-debian-base branch July 21, 2023 06:52
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

@dims dims Awaiting requested review from dims

Assignees

@dims dims

@ameukam ameukam

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/test cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. 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/testing Categorizes an issue or PR as relevant to SIG Testing. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
[sig-release] Bug Triage
Archived in project
Milestone
v1.28
Development

Successfully merging this pull request may close these issues.

[Failing Test] /var/lib/docker/tmp/buildkit-mount2116837088/lib64: no such file or directory
5 participants
@saschagrunert @k8s-ci-robot @ameukam @dims @neolit123