CRD in Operator 0.25.5 have invalid value for watched namespaces

[morkalfon]

After upgrading from ClickHouse Operator 0.25.3 to 0.25.5, our previous ClickHouseOperatorConfiguration manifests—valid and working on 0.25.3—are now rejected due to a breaking change in the CRD schema for the watch.namespaces field.

This is a YAML snippet of the relevant configuration (which worked in v0.25.3):

apiVersion: clickhouse.altinity.com/v1
kind: ClickHouseOperatorConfiguration
# ...
spec:
  # ...
  watch:
    namespaces:
      - clickhouse

• In 0.25.3, the CRD defines spec.watch.namespaces as an array of strings.
• In 0.25.5, this changed to an object (type: object), breaking compatibility with configurations that previously used the old array format or documentation that expected one behavior.

CRD change that caused breakage:

• 0.25.3 CRD lines:

namespaces:
  type: array
  description: "List of namespaces where clickhouse-operator watches for events."
  items:
    type: string

• 0.25.5 CRD lines:

namespaces:
  type: object
  description: "List of namespaces where clickhouse-operator watches for events."
  x-kubernetes-preserve-unknown-fields: true

[Slach]

accodring to https://github.com/Altinity/clickhouse-operator/blame/50e8d0490678185f290336acbdfbf9534d60367d/deploy/operator/parts/crd.yaml#L7160

it looks like was bugfix, and before just not works

how did you test array format in 0.25.3?
did you try to add custom resources in other namespaces then clickhouse?
which namespace do you use to install clickhouse-operator?

[morkalfon]

@Slach See example in the above message.

[morkalfon]

@Slach Let me know if you need more information from me. We deploy the operator to the clickhouse namespace and it failed to do so with this new CRD. This is the error we were getting when applied: Invalid value: "array": spec.watch.namespaces in body must be of type object: "array"

[Slach]

@morkalfon i read your example

i mean following:
your CRD in "old" format looks like just didn't work
i mean it loaded without error
but not resticted anything

if clickouse-operator deployment and kind: ClickHouseInstallation custom resources in the same clickhouse namespace, then clickouse-operator will watch CHI in the same namespace by default

so, you just need change your kind: ClickHouseOperatorConfiguration to new format to apply changes

is anything else what do you need to change with your kind: ClickHouseOperatorConfiguration?

[morkalfon]

@Slach I'm not sure I'm following you. We want to supply a list of namespaces to be watched by the operator, so how it can be done in the new CRD:

clickhouse-operator/deploy/operatorhub/0.25.5/clickhouseoperatorconfigurations.clickhouse.altinity.com.crd.yaml

Lines 51 to 58 in 50e8d04

	watch:
	type: object
	description: "Parameters for watch kubernetes resources which used by clickhouse-operator deployment"
	properties:
	namespaces:
	type: object
	description: "List of namespaces where clickhouse-operator watches for events."
	x-kubernetes-preserve-unknown-fields: true

what should be the new format?
In regards to what else do we need to change in kind: ClickHouseOperatorConfiguration we do change several things there. It's not just that.

[sunsingerus]

what should be the new format?

clickhouse-operator/docs/chi-examples/70-chop-config.yaml

Lines 11 to 19 in 50e8d04

	watch:
	# List of namespaces where clickhouse-operator watches for events.
	# Concurrently running operators should watch on different namespaces.
	# IMPORTANT
	# Regexp is applicable.
	namespaces:
	include: []
	exclude: []

[morkalfon]

I think we tried that already and it failed.

[janeklb]

I'm also dealing with the same issue in my operator installation and it's been especially annoying as I've been jumping between version 0.25.3 and 0.26.+ while testing some things.

The example format that @sunsingerus posted works fine, it's just that you have to apply it after you upgrade the operator's CRDs. In turn this makes managing the clickhouse-operator deployment a little annoying when you're using a flux HelmRelease because afaict you will end up with reconciliation errors no matter what you do. Actually, i think it could be avoided if you used flux's dependsOn and manage the operator and the operator configuration in separate kustomizations.. but imo that's not ideal.

eg.

• starting with operator chart 0.25.3 w/ working/valid ClickHouseOperatorConfiguration
• if you try to upgrade chart to 0.25.5+ and and configuration to accommodate the schema changes in one go, the dry-run will reject because the CRDs will still be on 0.25.3 and the new configuration shape will be invalid
• if you just upgrade the chart to 0.25.5 then after the upgrade is complete it will complain that the the existing ClickHouseOperatorConfiguration resource cannot be parsed by the operator because it doesn't follow the expected CRD shape

eg. for 0.25.3 we have

apiVersion: "clickhouse.altinity.com/v1"
kind: "ClickHouseOperatorConfiguration"
metadata:
  name: "chop-config-01"
spec:
  ################################################
  ##
  ## Watch Section
  ##
  ################################################
  watch:
    # List of namespaces where clickhouse-operator watches for events.
    # Concurrently running operators should watch on different namespaces
    #namespaces: ["dev", "test"]
    namespaces: []

but for 0.25.5 we have:

apiVersion: "clickhouse.altinity.com/v1"
kind: "ClickHouseOperatorConfiguration"
metadata:
  name: "chop-config-01"
spec:
  ################################################
  ##
  ## Watch Section
  ##
  ################################################
  watch:
    # List of namespaces where clickhouse-operator watches for events.
    # Concurrently running operators should watch on different namespaces.
    # IMPORTANT
    # Regexp is applicable.
    namespaces:
      include: []
      exclude: []

this is a breaking change (since it seems like the old "shape" is not supported by operator 0.25.5+)

I think the solution for these kinds of breaking changes is to release new CRD apiVersions and then support both in the operator for some time allowing customers to upgrade first the operator+CRDs and then their corresponding CRs. So for example 0.25.5 should have released a new apiVersion:

apiVersion: "clickhouse.altinity.com/v2" # <-- NOTICE v2
kind: "ClickHouseOperatorConfiguration"
metadata:
  name: "chop-config-01"
spec:
  ################################################
  ##
  ## Watch Section
  ##
  ################################################
  watch:
    # List of namespaces where clickhouse-operator watches for events.
    # Concurrently running operators should watch on different namespaces.
    # IMPORTANT
    # Regexp is applicable.
    namespaces:
      include: []
      exclude: []

[sunsingerus]

The example format that @sunsingerus posted works fine, it's just that you have to apply it after you upgrade the operator's CRDs.

Correct
The operator supports both formats and go code is backward compatible - can read any formats. However there can be obstacles with helm CRD upgrade