habitat-sh / habitat-operator

A Kubernetes operator for Habitat services
Apache License 2.0
60 stars 17 forks source link

[spec][backwards incompatible] drop support for v1beta1 #331

Closed surajssd closed 6 years ago

surajssd commented 6 years ago

This commit removes the support for v1beta1 resource type. The only supported version is only v1beta2.

krnowak commented 6 years ago

There seem to be a misunderstanding what this task was about. And the fact that we have this customVersion hack does not really help here.

So, so far our CRD has version habitat.sh/v1beta1. At some point we wanted to make an incompatible change (we switched from using Deployments to StatefulSets), so we wanted to introduce version habitat.sh/v1beta2. That didn't work - k8s at the time didn't have support for multiple versions of the CRD. It means that registering multiple versions failed and we needed to have it, because we still wanted to support the old version (with Deployments).

That's why we came up with the customVersion thing. It is a toplevel field which is our hacky take on CRD versioning. So if the field is absent in the manifest or if it is equal to v1beta1 then we expect yamls like:

apiVersion: habitat.sh/v1beta1
…
customVersion: v1beta1 # or unspecified
spec:
  image: habitat/redis-hab
  count: 1
  …

But if the customVersion field is equal to v1beta2 then we expect something like:

apiVersion: habitat.sh/v1beta2
…
customVersion: v1beta2
spec:
  v1beta2:
    image: habitat/redis-hab
    count: 1
    …

I suppose if we wanted to make another incompatible change in the manifest, we would add v1beta3 as a possible value for customVersion and v1beta3 field under spec, just like for v1beta2.

At some point we declared v1beta1 customVersion as deprecated and now we want to remove it. This currently means that customVersion field must be specified and must be equal to v1beta2 (until we introduce something like v1beta3 then this value would also be valid). So unspecified customVersion field or customVersion = v1beta1 are not supported anymore. This also means that the official CRD version is still v1beta1.

This customVersion hack is a thing we will need to keep until all versions of k8s the operator supports support CRD versioning. Basic form of CRD versioning arrived to k8s 1.11, not sure if it is in any way usable for us - I haven't yet investigated that. So you can see that this hack will stay with us a little while longer.

So about your changes: there is no need to change the CRD version. Keeping the CRD version intact should drop the need for renaming the directories, fixing the examples (the v1beta1 example still needs to be dropped though), resource names in tests, import paths in source files and some paths in bash scripts.

Oh, and thanks for the tests, they are always useful!

surajssd commented 6 years ago

@krnowak made changes as you requested PTAL!

surajssd commented 6 years ago

@krnowak all changes done!

krnowak commented 6 years ago

LFAD, thanks!