Allow override of registry and tag for Calico images

[hakman]

/hold for feedback
/cc @johngmyers @olemarkus @rifelpet

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [hakman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[seh]

I've tested this patch atop the "release-1.19" branch, and found that it works well.

[seh]

Do any of the nominated reviewers have any suggestions or objections here?

[rifelpet]

I'm not a huge fan of the inconsistency we're introducing between other CNIs and addons, though after looking into it the other CNIs are already inconsistent in their API field naming:

AWS VPC: .ImageName
Cilium:  .Version
Weave:   .Version

I know we've been waiting for addon operators for a long time now, but I think as long as we have a clear transition path from this to addon operators then it should be okay.

/lgtm

[hakman]

/hold

[hakman]

Thanks @rifelpet. You were right, I should have used .Version.
/hold cancel

[rifelpet]

👍🏻 I'm not a huge fan of Version because it suggests that kops will install the entire manifest associated with the requested version, when in reality it will use the manifest associated with kops' default version (for the k8s version) and then only override the image tags with the requested version. This could lead to RBAC issues or anything else that could stem from mismatched manifests and images.

but I think consistency is key here

/lgtm

[hakman]

Yes, will have to add some docs for this later and explain that version should be used with "care". :)

[olemarkus]

FWIW with Cilium we do try to apply the best manifest for the version given.

[hakman]

Could try that at some point I guess :). It is not a bad idea, just much more to maintain. Also, Calico releases much more versions.

[hakman]

/retest

[seh]

Trying to use this feature today, I once again left the leading 'v' character off the value I supplied to the "spec.networking.calico.version" field. Had the field been called "tag" or something other than "version," I probably would stop making that mistake.

[olemarkus]

You can copy the validation we have for the same thing in cilium :p

[hakman]

The idea was to not limit it to tags starting with v. Let's not add any extra validation :).

[seh]

Yes, the same problem comes up in many places that deal with semantic versions, and debate whether or not the leading 'v' is part of the version. Here, I think of the version as "3.17.3," and I know that the container image tag is the prefixed "v3.17.3."

The problem for me is the field name, not the values it accepts. I like that it accepts anything, because there's a lot of freedom for image tags. It's just misleading to suggest that it's asking for a version.