Feat: add common_crds role and Prometheus Operator CRDs installation

[tico88612]

What type of PR is this?

/kind feature

What this PR does / why we need it:

The use case proposed by the Post kubeadm hook (#12408) is to install Prometheus CRDs, such as ServiceMonitor. Most packages commonly use Prometheus Operator CRDs for monitoring, so this PR proposal is installed together with the Gateway API and merged into common_crds.

Which issue(s) this PR fixes:

Related #12408

Special notes for your reviewer:

The Prometheus Operator CRDs are commonly used for monitoring and are used by some CNIs (such as Cilium). Kubespray can be installed first, and the subsequent installation of the Prometheus operator can be handled by the user (or later extensions).

/label tide/merge-method-merge

Does this PR introduce a user-facing change?:

Added Prometheus Operator CRDs installation

[tico88612]

/hold
Preventing accidental merges, need review & discussion

[rarescosma]

This looks very promising, good job!

One question though: would it be possible to check if the CRD is installed already, and skip the install in this case? (say somewhere in a layer above Kubespray we change the CRD version, then we wouldn't want subsequent Kubespray runs overwriting it)

See this comment on the generic hooks PR too: #12408 (comment)

[tico88612]

I remember that upgrade is not yet integrated with CRDs (that is, upgrading a cluster does not include installing or upgrading CRDs)

[rarescosma]

I remember that upgrade is not yet integrated with CRDs (that is, upgrading a cluster does not include installing or upgrading CRDs)

Hmmm, I don't think this is about upgrades, we sort of run the cluster.yml playbook repeatedly. On subsequent runs, if the CRDs are already present in the cluster, we'd like to skip re-installing them.

Maybe something like this:

    - name: Apply ServiceMonitors CRD manifest
      shell: >-
        {{ kubectl }} get crd servicemonitors.monitoring.coreos.com \
        || {{ kubectl }} apply -f {{ temp_dir.path }}/crd-servicemonitors.yaml
      args:
        executable: /bin/bash
      run_once: true

Maybe expose it through another flag: ..._skip_if_found_....

That would make this feature actually usable for us, remove the need of generic hooks and make everything simpler 🥳

WDYT?

[tico88612]

@rarescosma I changed the state to present, I think this is what you want.

When set to present, an object will be created, if it does not already exist.

[tico88612]

/unhold

[rarescosma]

@rarescosma I changed the state to present, I think this is what you want.

When set to present, an object will be created, if it does not already exist.

Lovely, I'll try backporting this onto our 2.28 tracking branch and give it a go next week. Thank you!

[tico88612]

@MrFreezeex could you help me take a look at this? thanks!

[MrFreezeex]

Thanks that look promising!
The only thing that might be a bit weird to me is that we need to duplicate those default variable in multiple places (especially those version number unfortunately) but this is the same for the existing gateway-api crds so I wouldn't block that pr on that

I can add the lgtm label if you want me to or wait in case you want additional reviews as you wish

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrFreezeex, tico88612

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [tico88612]

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

[tico88612]

Thanks that look promising! The only thing that might be a bit weird to me is that we need to duplicate those default variable in multiple places (especially those version number unfortunately) but this is the same for the existing gateway-api crds so I wouldn't block that pr on that

Yep, ideally, I would like to write the CRD version in separate files, but obviously the download module will need to be used. We may need to discuss how to proceed.

I can add the lgtm label if you want me to or wait in case you want additional reviews as you wish

If you think it's okay to merge, I respect the opinion of each reviewer.

[VannTen]

The common CRDs idea looks smart to me, some comments.

[VannTen]

I somehow forgot that the download rewrite is still WIP 😅

[tico88612]

/retest

[tico88612]

/retest

[VannTen]

Can you just add the prometheus_crds_enable: true in one (or several) test cases so we exercise the code in CI ? Apart from that, looks OK to me.

[tico88612]

Can you just add the prometheus_crds_enable: true in one (or several) test cases so we exercise the code in CI ? Apart from that, looks OK to me.

Added it.

[VannTen]

/lgtm