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

Clarify CRI-O installation #10920

Merged
merged 2 commits into from Apr 9, 2020
Merged

Clarify CRI-O installation #10920

merged 2 commits into from Apr 9, 2020

Conversation

aanm
Copy link
Member

@aanm aanm commented Apr 9, 2020

This will help users deploying Cilium on CRI-O environments so they can easily choose if they should either use --set global.containerRuntime.integration=crio or not since we can't perform that detection automatically.

Fixes #10852

aanm added 2 commits April 9, 2020 18:29
@aanm
docs: remove restart cilium pod step in crio guide
86b260c 
Cilium does not require any container runtime endpoint to be available
so restarting Cilium is no longer required.

Signed-off-by: André Martins <andre@cilium.io>
@aanm
docs: add common CRIO issues
cb500d0 
This will help users deploying Cilium on CRI-O environments so they can
easily choose if they should either use `--set global.containerRuntime.integration=crio`
or not since we can't perform that detection automatically.

Signed-off-by: André Martins <andre@cilium.io>
@aanm aanm added pending-review release-note/misc This PR makes changes that have no direct user impact. labels Apr 9, 2020
@aanm aanm requested a review from a team as a code owner April 9, 2020 16:53
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.7.3 Apr 9, 2020
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Apr 9, 2020
@aanm aanm mentioned this pull request Apr 9, 2020
Closed
joestringer
joestringer approved these changes Apr 9, 2020
View reviewed changes
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

LGTM. Do you know which path is more common? Wondering if it's worth swapping the instructions around the other way if we think it's more likely that people don't need the container runtime integration flag.

@aanm
Copy link
Member Author

aanm commented Apr 9, 2020

LGTM. Do you know which path is more common? Wondering if it's worth swapping the instructions around the other way if we think it's more likely that people don't need the container runtime integration flag.

minikube would be more common I would say but that could give a false sense of security if a user tries this on minikube, does see anything wrong with it and then later on deploys it on a real cluster where everything starts failing. So I believe its better to fail earlier in this case.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.009%) to 46.675% when pulling cb500d0 on pr/fix-crio-docs into 7c88be7 on master.

@aanm aanm merged commit 14c9745 into master Apr 9, 2020
1.8.0 automation moved this from In progress to Merged Apr 9, 2020
@aanm aanm deleted the pr/fix-crio-docs branch April 9, 2020 19:29
@nebril nebril mentioned this pull request Apr 14, 2020
Merged
@joestringer joestringer moved this from Needs backport from master to Backport done to v1.7 in 1.7.3 Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

Assignees
No one assigned
Labels
release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.7.3
Backport done to v1.7
1.8.0
  
Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Not detecting BPF correctly when using cri-o
4 participants
@aanm @coveralls @joestringer @nebril