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

default cni config to list format #690

Merged
merged 2 commits into from
Apr 2, 2019
Merged

default cni config to list format #690

merged 2 commits into from
Apr 2, 2019

Conversation

MarkDeckert
Copy link
Contributor

Updated PR - this one also deletes existing *.conf files in cni config directory. These are some thoughts:

  1. This deletes existing *.conf file so it can safely be applied over existing kube-router and it will update the conf format.

  2. Although the kubelet only reads the first config in lexicographic order, there doesn't appear to be any limitation in the cni spec itself against having multiple configs. Therefore, the init container only deletes *.conf files if there was no pre-existing kube-router-installed 10-kuberouter.conflist file. The assumption is the first install is in a prepared environment, but after that all bets are off.

  3. Existing *.conflist or other files are not deleted. If kube-router is deployed into an environment like this the assumption is that the environment is complex and carefully crafted. But, if it gets deployed without that extra care, it may break things but at least it doesn't irretrievably destroy the previous configs.

  4. Perhaps some intelligence could be added where the init/install process parses existing conf/conflist files and reworks/adds itself in seamlessly, but this seems out of scope for a simple deployment config change like this.

@MarkDeckert
Copy link
Contributor Author

#684

@murali-reddy
Copy link
Member

murali-reddy commented Mar 23, 2019

@MarkDeckert thanks for reworking on the PR. Sorry i should have caught this earlier. But "cniVersion":"0.6.0" is in valid, (you may be getting confused from CNI version with plugin versions)

See https://github.com/containernetworking/cni/blob/master/SPEC.md

Please leave CNI version to be 0.3.0 as its in https://github.com/cloudnativelabs/kube-router/blob/master/daemonset/kubeadm-kuberouter-all-features-hostport.yaml#L12

Kubelet wont accept 0.6.0 as CNI version

  ----     ------                  ----              ----                     -------
  Normal   Scheduled               43s               default-scheduler        Successfully assigned default/frontend-74b4665db5-j6bp4 to 192.168.56.102
  Warning  FailedCreatePodSandBox  41s               kubelet, 192.168.56.102  Failed create pod sandbox: rpc error: code = Unknown desc = [failed to set up sandbox container "5d8b7ac93b0f98608fb738481b694830ef2cb0d89739d02131665aa2a3a93418" network for pod "frontend-74b4665db5-j6bp4": NetworkPlugin cni failed to set up pod "frontend-74b4665db5-j6bp4_default" network: incompatible CNI versions; config is "0.6.0", plugin supports ["0.1.0" "0.2.0" "0.3.0" "0.3.1"], failed to clean up sandbox container "5d8b7ac93b0f98608fb738481b694830ef2cb0d89739d02131665aa2a3a93418" network for pod "frontend-74b4665db5-j6bp4": NetworkPlugin cni failed to teardown pod "frontend-74b4665db5-j6bp4_default" network: incompatible CNI versions; config is "0.6.0", plugin supports ["0.1.0" "0.2.0" "0.3.0" "0.3.1"]]
  Normal   SandboxChanged          3s (x4 over 41s)  kubelet, 192.168.56.102  Pod sandbox changed, it will be killed and re-created.

@MarkDeckert
Copy link
Contributor Author

@murali-reddy Thank you I shouldn't have added that change at the last minute. I have fixed the version at 0.3.0 now.

@murali-reddy murali-reddy merged commit c2f893f into cloudnativelabs:master Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants