[kube-prometheus-stack] - Include v1beta1 AlertmanagerConfig crd

[Allex1]

• fixes # Adds the v1beta1 AlertmanagerConfig crd to kps crds chart. Currently we need to sideload it

Checklist

• [x] DCO signed
• [x] Chart Version bumped
• [x] Title of the PR starts with chart name (e.g. [prometheus-couchdb-exporter])

[GMartinez-Sisti]

Hi @Allex1! Thanks for the PR. I have a couple of questions:

• Is this a fix or a feature? Feels more like a feature as you're adding something. Maybe bump minor instead?

• Will updating the chart update the CRD? If I recall correctly, helm won't update already installed CRDs, or since this is a new one it will?

[Allex1]

• Is this a fix or a feature? Feels more like a feature as you're adding something. Maybe bump minor instead?

Good point, done

• Will updating the chart update the CRD? If I recall correctly, helm won't update already installed CRDs, or since this is a new one it will?

It will not (I tested) . Not sure how to approach this. Will a minor version bump on the crd chart and warning in the NOTES.txt (to delete the crd manually from 1 ver to the next) be enough to get this to new users at least ?

[GMartinez-Sisti]

Actually, I totally forgot that there are more moving parts regarding this:

• we use CRDs from prometheus-operator-crds helm chart
• we run charts/kube-prometheus-stack/hack/update_crds.sh to update it on this chart

This is going to be lost next time we run a sync if we don't do the right process now. Asking for help regarding this process to @jkroepke 🙏

[jkroepke]

@GMartinez-Sisti I created a follow-up issue #3941 to unblock this PR

[Allex1]

• we use CRDs from prometheus-operator-crds helm chart

1. This is new to me, why are there 2 crd charts that need separate maintaining ?
2. Also any idea why there are 2 crd folders in prometheus-operator repo ?
3. Can we just use the prometheus-crd-full folder instead of prometheus-operator-crd and update the sync script ?
4. Finally is my proposed solution for crd upgrade acceptable to get this pr unblocked?

[jkroepke]

This is new to me, why are there 2 crd charts that need separate maintaining ?

This is because the helm maintainer are unable to integrate CRD handing into Helm.

kube-prometheus-stack has the CRDs inside the crd/ folder while the secondary chart has the CRDs in the template directory.

Finally is https://github.com/prometheus-community/helm-charts/pull/3938#issuecomment-1780867145 for crd upgrade acceptable to get this pr unblocked?

Nothing needs to be decided in this PR, follow up discussion is in #3941. Please continue discussion around CRD handing there.

Thanks

[GMartinez-Sisti]

@Allex1 #3941 has been merged. Please rebase and fix conflicts, if you need to update something elsewhere the CI job will fail and let you know.

[jkroepke]

It seems like v1beta1 is not released yet.

https://github.com/prometheus-operator/prometheus-operator/issues/4677

[Allex1]

It seems like v1beta1 is not released yet.

@GMartinez-Sisti @jkroepke any idea about the reasons for not being officially released? Why is the crd included in the operator examples/crds then? We're using v1beta1 for a while now and would like to streamline the upgrade process. What's the recommendation ? Should we revert to v1alpha1 until the beta version is officially released ? Updated pr in order to work with the new crd...

[jkroepke]

Why is the crd included in the operator examples/crds then?

Where did you see this?

Looking at https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/main/example/prometheus-operator-crd/monitoring.coreos.com_alertmanagerconfigs.yaml there is only v1alpha1

[Allex1]

HERE

[jkroepke]

any idea about the reasons for not being officially released?

Ref: https://github.com/prometheus-operator/prometheus-operator/pull/4709/commits/582c845cc62f494279b97a4b4b321e7d9852c322

Enabling by default AlertmanagerConfig v1beta1 by default means that users would have to configure the conversion webhook and it must be performed in advance or at the same time users upgrade to the latest operator version. To offer a smoother transition, we offer AlertmanagerConfig v1beta1 as an opt-in feature: it's neither included in the bundle.yaml file nor in the example/prometheus-operator-crd/ manifests.

People that want to enable v1beta1 should use the example/prometheus-operator-crd-full manifests.

I'm fine to include v1beta1, but atm I'm not sure how to configure the conversion webhook which seems mandatory.

[Allex1]

@jkroepke Thanks, that provides more context. You make a good point about the conversion webhook but I don't think it's in scope of this pr. We just want to allow kps users to be able to choose between v1alpha and v1beta versions of the CRD(by deploying both as part of kps) not to convert their existing resources.

[jkroepke]

We just want to allow kps users to be able to choose between v1alpha and v1beta versions of the CRD(by deploying both as part of kps) not to convert their existing resources.

This is not supported in Kubernetes. Only one version is supported for storage.

https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definition-versioning/#configure-customresourcedefinition-to-use-conversion-webhooks

    # One and only one version must be marked as the storage version.
    storage: true

Looking at

HERE

its defined that v1alpha1 is marked a stored: true and v1beta1 is marked as storage: false.

---

Did you ever test, if you can create v1beta1 AlertmanagerConfigs? I have the feeling that the web converation webhook is mandory for v1beta1. At the current state, its expected that v1beta1 objects are converted into v1alpha1 unless its "released".

• https://github.com/prometheus-operator/prometheus-operator/blob/3c13f057abc914f841f8675eefab8e46a8ad73ae/Documentation/user-guides/webhook.md#converting-alertmanagerconfig-resources
• And a follow-up PR should include the validation WebHook for AlertmanagerConfig (https://github.com/prometheus-operator/prometheus-operator/blob/3c13f057abc914f841f8675eefab8e46a8ad73ae/Documentation/user-guides/webhook.md#alertmanagerconfig)

[Allex1]

@jkroepke thanks for the context:

Looking at https://raw.githubusercontent.com/prometheus-operator/prometheus-operator/main/example/prometheus-operator-crd/monitoring.coreos.com_alertmanagerconfigs.yaml there is only v1alpha1

Also present in the official docs

Did you ever test, if you can create v1beta1 AlertmanagerConfigs? I have the feeling that the web converation webhook is mandory for v1beta1. At the current state, its expected that v1beta1 objects are converted into v1alpha1 unless its "released".

We're using this exact setup and have created multiple v1beta1 amcfg resources (starting ~ 6m - 1 year ago) which are not converted to v1alpha , they respect the v1beta1 format , the config gets injected into AM and were validated to trigger alerts on different integrations.

[jkroepke]

@GMartinez-Sisti Whats your opinion here? There is no official statement about the v1beta1 Alertmanager CRD yet (https://github.com/prometheus-operator/prometheus-operator/issues/4677) nor its included in the in the prometheus-operator-crd, only in prometheus-operator-crd-full as preview.

I have the feeling that its not ready to use for everyone.

Additionally, one one is blocked here. If someone wants to test v1beta1 CRD, it can be installed manually.