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

[CI Job failure] Fix job containerd-e2e-ubuntu #107832

Merged
merged 1 commit into from
Feb 3, 2022
Merged

[CI Job failure] Fix job containerd-e2e-ubuntu #107832

merged 1 commit into from
Feb 3, 2022

Conversation

adisky
Copy link
Contributor

@adisky adisky commented Jan 28, 2022 •
edited

Signed-off-by: Aditi Sharma adi.sky17@gmail.com

What type of PR is this?

/kind failing-test

What this PR does / why we need it:

A possible fix for https://testgrid.k8s.io/sig-node-containerd#containerd-e2e-ubuntu
jobs which require containerd installation from ubuntu repos always set this

KUBE_UBUNTU_INSTALL_CONTAINERD_VERSION
KUBE_INSTALL_RUNC_VERSION

https://cs.k8s.io/?q=KUBE_UBUNTU_INSTALL_CONTAINERD_VERSION&i=nope&files=&excludeFiles=&repos=

for this job we install containerd from containerd repo
https://github.com/kubernetes/test-infra/blob/master/config/jobs/kubernetes/sig-node/containerd.yaml#L9-#L10
the order of installation might be different for both methods , so checking containerd installation at this point for this job might be causing the failure.

Also before this PR got merged the flag container_runtime=remote was preventing to reach this check for this job
https://github.com/kubernetes/kubernetes/pull/107663/files#diff-dcbe601f5e14c1d9d805d0daf3629e02d1e1ea2be615f6e7f3036ca90a25c6e4L597-L608

EDIT: on digging further with @bobbypage it turned out that ctr is not in the path while doing these checks, so sourcing containerd env if present, see discussion on the issue page for more details

Which issue(s) this PR fixes:

ref #107800

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

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

@k8s-ci-robot k8s-ci-robot added kind/failing-test Categorizes issue or PR as related to a consistently or frequently failing test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. 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. 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. release-note-none Denotes a PR that doesn't merit a release note. area/provider/gcp Issues or PRs related to gcp provider sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. 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. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 28, 2022
@adisky
Copy link
Contributor Author

adisky commented Jan 28, 2022

/sig node
/cc @bobbypage @SergeyKanzhelev @endocrimes

@k8s-ci-robot k8s-ci-robot added the sig/node Categorizes an issue or PR as relevant to SIG Node. label Jan 28, 2022
@adisky adisky force-pushed the fix-ubuntu-containerd-install branch from 07a31f4 to 064b1a6 Compare January 28, 2022 06:44
@k8s-ci-robot k8s-ci-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 28, 2022
@adisky adisky force-pushed the fix-ubuntu-containerd-install branch from 064b1a6 to 849ad2d Compare January 28, 2022 06:45
@adisky adisky changed the title [CI Job failure] Verify containerd presence if installed [CI Job failure] Source containerd environment Jan 28, 2022
@adisky adisky changed the title [CI Job failure] Source containerd environment [CI Job failure] Fix job containerd-e2e-ubuntu Jan 28, 2022
endocrimes
endocrimes approved these changes Jan 28, 2022
View reviewed changes
Copy link
Member

@endocrimes endocrimes left a comment

Choose a reason for hiding this comment

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

/lgtm

thanks for fixing this 🎉 (and thanks @bobbypage for testing), one of the fun parts of OSS is when stuff happens while 😴.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2022
SergeyKanzhelev
SergeyKanzhelev approved these changes Jan 28, 2022
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

/priority important-soon
/triage accepted
/approve
/hold

(hold for the comment update)

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 28, 2022
@SergeyKanzhelev
Copy link
Member

/retest

@bobbypage
Copy link
Member

/lgtm

Thanks for making the fix!

@adisky
Source containerd env for repo installation
a251acd 
Signed-off-by: Aditi Sharma <adi.sky17@gmail.com>
@adisky adisky force-pushed the fix-ubuntu-containerd-install branch from 849ad2d to a251acd Compare January 31, 2022 04:00
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2022
endocrimes
endocrimes approved these changes Jan 31, 2022
View reviewed changes
Copy link
Member

@endocrimes endocrimes left a comment

Choose a reason for hiding this comment

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

/lgtm

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

adisky commented Feb 1, 2022

/assign @cheftako @dchen1107
for cluster/gce/gci/ approval

@SergeyKanzhelev SergeyKanzhelev moved this from Triage to PRs - Needs Approver in SIG Node CI/Test Board Feb 2, 2022
@dchen1107
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adisky, dchen1107, endocrimes, SergeyKanzhelev

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 Feb 2, 2022
@adisky
Copy link
Contributor Author

adisky commented Feb 3, 2022

/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 Feb 3, 2022
@k8s-ci-robot k8s-ci-robot merged commit 1f88942 into kubernetes:master Feb 3, 2022
SIG Node CI/Test Board automation moved this from PRs - Needs Approver to Done Feb 3, 2022
@k8s-ci-robot k8s-ci-robot added this to the v1.24 milestone Feb 3, 2022
@endocrimes endocrimes mentioned this pull request Feb 7, 2022
Closed
@adisky adisky deleted the fix-ubuntu-containerd-install branch April 4, 2022 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@endocrimes endocrimes endocrimes approved these changes

@SergeyKanzhelev SergeyKanzhelev SergeyKanzhelev approved these changes

@roycaihw roycaihw Awaiting requested review from roycaihw

@bobbypage bobbypage Awaiting requested review from bobbypage

@dims dims Awaiting requested review from dims

Assignees

@bobbypage bobbypage

@cheftako cheftako

@endocrimes endocrimes

@dchen1107 dchen1107

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/provider/gcp Issues or PRs related to gcp provider 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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/node Categorizes an issue or PR as relevant to SIG Node. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
SIG Node CI/Test Board
Done
SIG Node CI/Test Board
Archived in project
Milestone
v1.24
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@adisky @SergeyKanzhelev @bobbypage @dchen1107 @k8s-ci-robot @endocrimes @cheftako