Please remove `nat-outgoing` from `crd.projectcalico.org_ippools.yaml`

[zachfi]

Some tooling to generate Jsonnet from CRDs breaks on the ippools CRD due to the nat-outgoing key, which has been deprecated. Can this be removed? I believe this wold unblock ippools handling and allow jsonnet users to manage those resources.

https://github.com/jsonnet-libs/calico-libsonnet

[caseydavenport]

Is the problem that the field is marked as deprecated? Or is it something else?

Removing that field might break old clusters that existed prior to the new field, so I'm hesitant to remove it outright.

[zachfi]

I'm not sure, but I believe it may be due to the hyphen in the name, or perhaps that if the name is normalized with natOutgoing, then there would be one less than len() in the array.

I was noticing this from a comment someone left in the automation from a while ago and figured that if the deprecation had been in place long enough, it might be safe to remove. But you all will know more about that in terms of when to feel comfortable removing a deprecation.

[lwr20]

Removing that field might break old clusters that existed prior to the new field, so I'm hesitant to remove it outright.

I discussed with Casey. The worry is that anyone who created an ippool using the version that introduced that field and then didn't touch it again ever wouldn't have had this fixed up.

Perhaps we need some upgrade code in calico-kubecontrollers to spot if this field is in use and if so move the value to the new field. We'd also need to implement an upgrade policy (specifying how many calico releases its OK to skip) to ensure that users will go through a version with the migration code in it. Only after that could we remove the field.

[lwr20]

@xaque208 What are jsonnet libraries used for? i.e. what are we breaking by not fixing this?

[zachfi]

Jsonnet is a programming language for programming json output, generally speaking. One common use case is to use it like Helm, but more expressive to create the necessary kubernetes resources which are then diff'ed and applied to clusters. Some automation that the community uses takes the CRDs that you all produce, and uses them to generate libraries that can be used in these jsonnet programs to produce output.

https://jsonnet-libs.github.io/calico-libsonnet/3.20/crd/v1/

What is missing from there is the ippools, which only really matters when you init a new cluster, which happens from time to time in a dev context.

So to give an example, I use the resources above in a little jsonnet to configure calico on my clusters.

local calico = import 'calico.libsonnet';
local k = import 'ksonnet-util/kausal.libsonnet';

{
  local bgpConfiguration = calico.crd.v1.bgpConfiguration,
  local bgpPeer = calico.crd.v1.bgpPeer,

  bgp_configuration:
    bgpConfiguration.new('default')
    + bgpConfiguration.spec.withAsNumber(4279719999)
    + bgpConfiguration.spec.withServiceClusterIPs([
      { cidr: '10.55.0.0/16' },
      { cidr: 'fccb::/112' },
    ])
    + bgpConfiguration.spec.withServiceExternalIPs([
      { cidr: '10.98.0.0/28' },
      { cidr: 'fccc::/120' },
    ])
    + bgpConfiguration.spec.withNodeToNodeMeshEnabled(false),

It gets pretty neat because you can avoid repeating yourself and make functions etc like so.

  not_me_peer(name, as, ip4, ip6)::
    bgpPeer.new('%s-inet4' % name)
    + bgpPeer.spec.withAsNumber(as)
    + bgpPeer.spec.withPeerIP(ip4)
    + bgpPeer.spec.withNodeSelector("kubernetes.io/hostname != '%s'" % name)

    + bgpPeer.new('%s-inet6' % name)
    + bgpPeer.spec.withAsNumber(as)
    + bgpPeer.spec.withPeerIP(ip6)
    + bgpPeer.spec.withNodeSelector("kubernetes.io/hostname != '%s'" % name),

Anyway, that's jsonnet -> yaml -> cluster. So in a new cluster context, there is one ippoool resource that needs to be applied outside of the automation that jsonnet provides. I'm glossing over some details, because the rabbit hole is deep.

I do understand though about not wanting to break existing users. Not sure what the upgrade cadence is for your general use case, but I've worked with network engineers long enough to know how they feel about upgrades sometimes :)

[zachfi]

I also recognize that if you add code to tell if users have upgraded, they probably need to upgrade for you to see that, but perhaps you had something else in mind. Always tough to know when users upgrade, but deprecations are useful to let folks know that they should be upgrading, at least for those users who are paying attention.