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 support for cilium network provider #265

Merged

Conversation

SimonHoenscheid
Copy link
Contributor

No description provided.

@carabasdaniel
Copy link
Contributor

Hello Simon,

Could you provide an update for the kube_tool to be able to generate the configuration for cilium also ?

For reference: https://github.com/puppetlabs/puppetlabs-kubernetes/blob/master/tooling/kube_tool/other_params.rb#L21

@SimonHoenscheid
Copy link
Contributor Author

sure!

@carabasdaniel
Copy link
Contributor

Hi @SimonHoenscheid,

Could you please correct this issue with the added version parameter for cni_provider_version

/etc/k8s/kube_tool/other_params.rb:8:in `create': wrong number of arguments (given 8, expected 9) (ArgumentError)
	from /etc/k8s/kube_tool.rb:67:in `build_hiera'
	from /etc/k8s/kube_tool.rb:81:in `<main>'

This is the call that has the missing parameter:
https://github.com/puppetlabs/puppetlabs-kubernetes/blob/master/tooling/kube_tool.rb#L63

I think you might want to set this parameter as optional regarding the fact that for weave and flannel we use the Kubernetes version.

Thanks.

@SimonHoenscheid
Copy link
Contributor Author

@carabasdaniel I hope the syntax for making the parameter optional is the right one

tooling/Dockerfile Outdated Show resolved Hide resolved
tooling/kube_tool.rb Outdated Show resolved Hide resolved
@carabasdaniel
Copy link
Contributor

@SimonHoenscheid I've tested it and left a few comments. It looks good, but those two issues need to be fixed. When doing the changes please test if the docker image generates the values as intended for two cni-providers (example: weave and cilium).
To build the docker image locally you can use: docker build -t kubetool:latest .
To test for weave you can use for example this command: docker run --rm -v $(pwd):/mnt -e OS=ubuntu -e VERSION=1.13.5 -e CONTAINER_RUNTIME=docker -e CNI_PROVIDER=weave -e ETCD_INITIAL_CLUSTER=kube-master:172.20.20.10 -e ETCD_IP="172.20.10.8" -e KUBE_API_ADVERTISE_ADDRESS="127.0.0.1:443" -e INSTALL_DASHBOARD=true kubetool:latest

@carabasdaniel
Copy link
Contributor

👍 LGTM

@carabasdaniel carabasdaniel merged commit 67c03cc into puppetlabs:master Jun 5, 2019
@SimonHoenscheid SimonHoenscheid deleted the add_cilium_support branch January 21, 2022 14:34
@xmulligan
Copy link

@SimonHoenscheid Thank you for adding this support! Would you mind adding Puppet to the Cilium adopters list? https://github.com/cilium/cilium/blob/master/USERS.md

We need this for when we apply to graduate at the CNCF

@SimonHoenscheid
Copy link
Contributor Author

@xmulligan I am not a Puppet Employee and can not say if its ok for Puppet Inc. Please ask @nigelkersten @binford2k.
BTW I am currently reworking the code to use cilium-cli to bootstrap cilium.

@xmulligan
Copy link

@nigelkersten @binford2k can you help here?

@SimonHoenscheid glad to hear about cilium-cli ! 🐝

@nigelkersten
Copy link

nigelkersten commented Feb 25, 2022 via email

@xmulligan
Copy link

@nigelkersten just wanted to circle back on this :) have you found out who the right person it?

@binford2k
Copy link
Contributor

As far as I can tell, we don't actually use Cilium ourselves, we just build some tools that allow our users to use Cilium. It would not be appropriate to add ourselves to that list at this time.

Thanks for following up though!

@xmulligan
Copy link

@binford2k Thanks for the update! Feel free to reach out if your users need help with Cilium :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants