Skip to content

ci: make docker images in ghaction#11693

Merged
borkmann merged 2 commits intomasterfrom
pr/build-images-ghaction
Jun 3, 2020
Merged

ci: make docker images in ghaction#11693
borkmann merged 2 commits intomasterfrom
pr/build-images-ghaction

Conversation

@nebril
Copy link
Copy Markdown
Member

@nebril nebril commented May 26, 2020

This extends the helm lint gh action by building the docker images and running simple ginkgo test against kind cluster.

@maintainer-s-little-helper
Copy link
Copy Markdown

Please set the appropriate release note label.

@coveralls
Copy link
Copy Markdown

coveralls commented May 26, 2020

Coverage Status

Coverage increased (+0.001%) to 36.917% when pulling 67d4e1a on pr/build-images-ghaction into 40b6f58 on master.

@nebril nebril force-pushed the pr/build-images-ghaction branch 26 times, most recently from 34cd0d8 to dd33d76 Compare May 27, 2020 12:31
@nebril
Copy link
Copy Markdown
Member Author

nebril commented May 28, 2020

test-net-next

@nebril
Copy link
Copy Markdown
Member Author

nebril commented May 28, 2020

test-runtime

Copy link
Copy Markdown
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

This is awesome!

Mostly nits from my side. Would be nice to update the name of the workflow though.

Comment thread .github/workflows/helm-check.yaml
Comment thread .github/workflows/helm-check.yaml Outdated
Comment thread test/helpers/kubectl.go
Comment thread test/helpers/kubectl.go
Comment thread test/k8sT/Conformance.go

err := kubectl.WaitforPods(helpers.DefaultNamespace, "", helpers.HelperTimeout)
ExpectWithOffset(1, err).Should(BeNil(), "connectivity-check pods are not ready after timeout")
})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I understand correctly, we could use the existing test above if we were able to deploy several nodes in GitHub Actions? I think @sayboras was working on this (cf. #11738).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Unfortunately this wouldn't really work for multi-node clusters, we would be running several Cilium agent instances on one kernel which would most probably wreak havoc in bpf filesystem and break everything, as we don't do any namespacing afaik (I don't know if this would even be possible).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pchaigno I saw this awesome PR, and thinking about trying to multi node in kind. It's for my own understanding only. There is one related PR #11715, which makes the connectivity check simpler as well. As @nebril mentioned, might need to relax some condition/scope to make it working with multiple nodes in kind :)

Copy link
Copy Markdown
Member

@pchaigno pchaigno May 29, 2020

Choose a reason for hiding this comment

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

we would be running several Cilium agent instances on one kernel which would most probably wreak havoc

I don't think that's an issue. We even have a GSG for a multi-node Kind setup.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yeah, I followed the above guide to setup my dev environment. My original plan is to use chart testing with custom values coming from ci directory (with the same value mentioned in guide). However, understanding from the team that having extra ci folder might not be idea, so might need to wait for helm/chart-testing#171 to be resolved first. @nebril is very kind to spend time explaining and pointing me to helm/chart-testing#171 🎉

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I totally missed this gsg guilde, thanks for this info!

Copy link
Copy Markdown
Member

@sayboras sayboras May 29, 2020

Choose a reason for hiding this comment

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

@nebril @pchaigno in that case, what do you think about this https://github.com/sayboras/cilium/pull/71/checks?check_run_id=720440156 ? FYI. I just copied details from this PR and GSG link

sayboras#71

@nebril nebril force-pushed the pr/build-images-ghaction branch from 2ee75ba to 0565839 Compare May 29, 2020 12:10
Comment thread test/helpers/kubectl.go Outdated
Comment thread test/helpers/kubectl.go Outdated
Comment thread examples/kubernetes/connectivity-check/Makefile Outdated
Comment thread .github/workflows/helm-check.yaml Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't we set the pull policy value with helm?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not with the action we are using here :/

@nebril nebril force-pushed the pr/build-images-ghaction branch from 0565839 to 2a5a5c9 Compare May 29, 2020 14:24
@nebril
Copy link
Copy Markdown
Member Author

nebril commented May 29, 2020

test-focus K8sConformance

@nebril nebril requested a review from aanm May 29, 2020 14:56
@nebril
Copy link
Copy Markdown
Member Author

nebril commented Jun 1, 2020

test-4.9

@christarazi christarazi added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jun 2, 2020
nebril added 2 commits June 3, 2020 12:28
Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
Make docker images
Import them into kind cluster
Install gingko
Run ginkgo test

Signed-off-by: Maciej Kwiek <maciej@isovalent.com>
@nebril nebril force-pushed the pr/build-images-ghaction branch from 2a5a5c9 to 67d4e1a Compare June 3, 2020 10:30
@nebril nebril added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. labels Jun 3, 2020
@nebril
Copy link
Copy Markdown
Member Author

nebril commented Jun 3, 2020

test-focus K8sConformance

@nebril
Copy link
Copy Markdown
Member Author

nebril commented Jun 3, 2020

hit vagrant provisioning flake fixed by #11858, rerunning conformance tests

@nebril
Copy link
Copy Markdown
Member Author

nebril commented Jun 3, 2020

test-focus K8sConformance

1 similar comment
@nebril
Copy link
Copy Markdown
Member Author

nebril commented Jun 3, 2020

test-focus K8sConformance

@borkmann borkmann merged commit 9056286 into master Jun 3, 2020
@borkmann borkmann deleted the pr/build-images-ghaction branch June 3, 2020 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants