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

Add CI for docker in Github Actions #1965

Closed
wants to merge 4 commits into from

Conversation

aojea
Copy link
Contributor

@aojea aojea commented Dec 7, 2020

It also fixes a bug where the images failed to run after they were created locally

When we added Support configuring a cgroup root for kubelet #1747 , we checked for podman or docker to remount the cgroups. However, docker in Github doesn't show docker in its cgroup path, causing that the script didn't remounted the cgroups subsystems under /kubelet, hence the kubelet, that is configured to use cgroup-root: /kubelet fails to start because there is nothing in that path.

grep /sys/fs/cgroup /proc/self/mountinfo | grep docker
root@kind-control-plane:/usr/local/bin# 

Fixes: #1969

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 7, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aojea
To complete the pull request process, please assign bentheelder after the PR has been reviewed.
You can assign the PR to them by writing /assign @bentheelder in a comment when ready.

The full list of commands accepted by this bot can be found 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 size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 7, 2020
@aojea
Copy link
Contributor Author

aojea commented Dec 7, 2020

/assign @BenTheElder

IP_FAMILY: ${{ matrix.ipFamily }}
steps:
- name: Check out code into the Go module directory
uses: actions/checkout@v2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BenTheElder
Copy link
Member

interesting that it does not break here 🤔

@aojea
Copy link
Contributor Author

aojea commented Dec 7, 2020

for reference, it fails here for the cgroup you can get the logs from here https://github.com/aojea/ovn-kubernetes/runs/1512853588?check_suite_focus=true with following error

Dec 07 19:16:44 ovn-control-plane kubelet[994]: F1207 19:16:44.170173 994 server.go:269] failed to run Kubelet: invalid configuration: cgroup-root ["kubelet"] doesn't exist
Dec 07 19:16:44 ovn-control-plane kubelet[994]: goroutine 1 [running]:

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 10, 2020
@aojea
Copy link
Contributor Author

aojea commented Dec 10, 2020

we have a reproducer and a job to test that we don´t break last stable version
https://github.com/kubernetes-sigs/kind/pull/1965/checks?check_run_id=1529704560

@BenTheElder
Copy link
Member

/retest

@aojea aojea force-pushed the dockerci branch 7 times, most recently from c87154a to faf7cd1 Compare December 15, 2020 14:21
continue-on-error: true

- name: Export logs
if: always()

Choose a reason for hiding this comment

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

Suggested change
if: always()
if: ${{ always() }}

Copy link

@lotharschulz lotharschulz Dec 15, 2020

Choose a reason for hiding this comment

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

The double curly braces syntax is used in line 56 as well and is in line with the documentation. If this suggestion is accepted the same change should apply to line 88.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, thanks

@@ -20,7 +20,7 @@ package nodeimage
const DefaultImage = "kindest/node:latest"

// DefaultBaseImage is the default base image used
const DefaultBaseImage = "kindest/base:v20201130-23777eca"
const DefaultBaseImage = "kindest/base:v20201204-49cad832"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ping @BenTheElder , however, this doesn't solve the problem

Copy link
Member

Choose a reason for hiding this comment

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

where did you pull that from? note: random images in this repo are not supported.
I dont think we missed any bumps merging.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pulled from here https://hub.docker.com/r/kindest/base/tags?page=1&ordering=last_updated , it says updated 11 days ago

@aojea
Copy link
Contributor Author

aojea commented Dec 15, 2020

/close
#1979

@k8s-ci-robot
Copy link
Contributor

@aojea: Closed this PR.

In response to this:

/close
#1979

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.

@@ -0,0 +1,86 @@
name: Compatibility
Copy link
Member

Choose a reason for hiding this comment

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

this we don't guarantee and was a red-herring

@@ -0,0 +1,95 @@
name: Docker
Copy link
Member

Choose a reason for hiding this comment

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

a variant of this we should probably keep somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kubelet: invalid configuration: cgroup-root ["kubelet"] doesn't exist
4 participants