search

vmware-archive / wardroom

A tool for creating Kubernetes-ready base operating system images.
Apache License 2.0
162 stars 44 forks source link

Updates to Calico CNI Deployment #137

Open joshrosso opened 5 years ago

joshrosso commented 5 years ago

I'd like to make the following changes to how wardroom does its Calico deployments.

Please let me know if they sound good and I will make the changes.

  1. Increase the version to 3.6.X from 3.3.X

  2. Store the calico manifest as a jinja template

  3. Add variables to ensure calico respects the variables set for the cluster's podSubnet as its IPV4 pool.

  4. Set IP-in-IP to crossSubnet (https://docs.projectcalico.org/v3.5/usage/configuration/ip-in-ip#configuring-crosssubnet-ip-in-ip) rather than the default always.

    a. This will ensure that IP-in-IP encapsulation is not used for all traffic, causing unnecessary overhead.

/cc @craigtracey @scottslowe

scottslowe commented 5 years ago

No objections to any of the suggested changes. Are you going to make these changes against master, and then we'll port over to the v1.13.x branch?

craigtracey commented 5 years ago

I am all for bumping versions of Calico. Which branches would you like to apply this to?

As for storing the Calico manifests as Jinja templates, I would rather that we not do that, as it will make things a lot more rigid/confusing than they are today. And, it would be another burden for us to maintain.

The repo does have a mechanism to manipulate any applied manifest (called modify_manifest), and I would prefer that we take that route...by documenting modify_manifest options that a user may employ.

craigtracey commented 5 years ago

As for podSubnet, we intentionally do not set this today, as this will vary wildly between deployments. Again, I think we should very clearly document how this modify_manifest library can be leveraged.

An example: https://github.com/heptiolabs/wardroom/blob/master/swizzle/examples/calico.yml#L15-L21

joshrosso commented 5 years ago

I’m not sure I personally agree with having a template-tized manifest being more confusing. The endpoint is a black box. And Calico may change what sits behind it. For example in 3.5, there is no longer an endpoint for RBAC, everything sits in 1 manifest.

However I am willing to concede that preference in favor of a well documented modify_manifest.

As for podSubnet, we intentionally do not set this today,

Right, so shouldn’t wardroom support users setting their podCIDR arbitrarily? And that value must propagate to the Calico yaml as it determines the IPV4 pool.

From: Craig Tracey notifications@github.com Sent: Friday, April 19, 2019 12:29 PM To: heptiolabs/wardroom Cc: joshrosso; Author Subject: Re: [heptiolabs/wardroom] Updates to Calico CNI Deployment (#137)

As for podSubnet, we intentionally do not set this today, as this will vary wildly between deployments. Again, I think we should very clearly document how this modify_manifest library can be leveraged.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/heptiolabs/wardroom/issues/137#issuecomment-484949588, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABPJV6NFE2ZGBXEPMTGLWG3PRHXORANCNFSM4HHE6FLQ.

joshrosso commented 5 years ago

Additionally, I (personally) only care about this change for the targeted 1.14 release. Backporting is not important IMO.

From: Josh Rosso joshrosso@gmail.com Sent: Friday, April 19, 2019 12:50 PM To: heptiolabs/wardroom; heptiolabs/wardroom Cc: Author Subject: Re: [heptiolabs/wardroom] Updates to Calico CNI Deployment (#137)

I’m not sure I personally agree with having a template-tized manifest being more confusing. The endpoint is a black box. And Calico may change what sits behind it. For example in 3.5, there is no longer an endpoint for RBAC, everything sits in 1 manifest.

However I am willing to concede that preference in favor of a well documented modify_manifest.

As for podSubnet, we intentionally do not set this today,

Right, so shouldn’t wardroom support users setting their podCIDR arbitrarily? And that value must propagate to the Calico yaml as it determines the IPV4 pool.

From: Craig Tracey notifications@github.com Sent: Friday, April 19, 2019 12:29 PM To: heptiolabs/wardroom Cc: joshrosso; Author Subject: Re: [heptiolabs/wardroom] Updates to Calico CNI Deployment (#137)

As for podSubnet, we intentionally do not set this today, as this will vary wildly between deployments. Again, I think we should very clearly document how this modify_manifest library can be leveraged.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/heptiolabs/wardroom/issues/137#issuecomment-484949588, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABPJV6NFE2ZGBXEPMTGLWG3PRHXORANCNFSM4HHE6FLQ.

craigtracey commented 5 years ago

Wardroom does support setting podCIDR arbitrarily, but there is no explicit check to ensure that Calico also sets that same CIDR. It has always been the intention that the user needs to validate their configuration before deployment. If we want to write a warning about the mismatch, I am fine with that.

Carrying a template means that we need to fast-follow releases from Calico, with no guarantee that our variables will stay in lock step with theirs. This leaves the user in a spot where a configuration that worked once, may not work again....and probably fails silently, as it is just a template. While modify_manifest does not have strict enforcement currently, it would be somewhat trivial to add a parameter that forces all rules to be evaluated and modified.

joshrosso commented 5 years ago

Wardroom does support setting podCIDR arbitrarily, but there is no explicit check to ensure that Calico also sets that same CIDR.

Are there cases where podCIDR for the k8s cluster should not be the CIDR used for Calico? Why not just reuse this variable in the Calico template (or always set it via modify_manifest?

Carrying a template means that we need to fast-follow releases from Calico

When a new Calico release happens, the new yaml needs to be copied over and the variables re-established (or someone does diff work). Having the template-tized manifest makes the ansible easier to reason with. This overhead is not substantial and better as having config management pull a manifest from the sky and run a dynamic mod on it probably won't fly in some cases / environments.

Again, just my view of the world. I am OK with strictly using modify_manifest. I do however feel more strongly about the podCIDR :smile: .

joshrosso commented 5 years ago

As part of this ticket, I'll provide documentation around modify_manifest.

We do need to find consensus around our philosophy for plumbing podCIDR.

@craigtracey @scottslowe