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

vagrant: Only set K8S_NODE_NAME if K8S=1 #11086

Merged
merged 1 commit into from Apr 22, 2020
Merged

vagrant: Only set K8S_NODE_NAME if K8S=1 #11086

merged 1 commit into from Apr 22, 2020

Conversation

jrajahalme
Copy link
Member

This allows Cilium to start without K8s in the Dev VM started with './contrib/vagrant/start.sh'.

Fixes: #11021
Signed-off-by: Jarno Rajahalme jarno@covalent.io

@jrajahalme
vagrant: Only set K8S_NODE_NAME if K8S=1
7f71ad7 
Fixes: #11021
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
@jrajahalme jrajahalme added kind/bug This is a bug in the Cilium logic. area/containerd release-note/misc This PR makes changes that have no direct user impact. labels Apr 21, 2020
@jrajahalme jrajahalme requested a review from a team as a code owner April 21, 2020 21:14
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Apr 21, 2020
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

This is not really tested in the CI, as if only affects the devVM started with contrib/vagrant/start.sh. Tested locally.

@pchaigno
Copy link
Member

pchaigno commented Apr 21, 2020 •
edited

Failed with weird error: nodeRegistration.criSocket: Invalid value: "unix:///run/containerd/containerd.sock": path is not absolute.

restart-ginkgo

pchaigno
pchaigno approved these changes Apr 21, 2020
View reviewed changes
Copy link
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.

Thanks Jarno!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 44.682% when pulling 7f71ad7 on pr/jrajahalme/vagrant-start-wo-k8s into 5370abe on master.

@joestringer
Copy link
Member

The criSocket failure seems more likely to be related to #11060, although that PR passed CI..

@jrajahalme
Copy link
Member Author

Ginkgo provisioning fail.

@jrajahalme
Copy link
Member Author

As said, this is ready to merge without passing the CI.

@jrajahalme jrajahalme added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Apr 22, 2020
@christarazi christarazi merged commit fff6d6c into master Apr 22, 2020
1.8.0 automation moved this from In progress to Merged Apr 22, 2020
@christarazi christarazi deleted the pr/jrajahalme/vagrant-start-wo-k8s branch April 22, 2020 01:01
@pchaigno
Copy link
Member

pchaigno commented Oct 5, 2020

The PR that introduced the bug was backported to v1.7 and v1.6, thus breaking the runtime1 VM there. The fix is really small and doesn't conflict, so I think it's worth backporting to v1.7 to ease backport tests (I usually spin up the runtime1 VM to have the appropriate Go version when I test-compile backport PRs).

I'm adding the needs-backport/1.7 label and I will add this commit to #13402.

@pchaigno pchaigno mentioned this pull request Oct 5, 2020
Merged
@christarazi christarazi mentioned this pull request Oct 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@aanm aanm aanm approved these changes

Assignees
No one assigned
Labels
kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@jrajahalme @pchaigno @coveralls @joestringer @aanm @christarazi @qmonnet