telco-hub: add first set of reference templates for the cluster-compare tool by jmontesi · Pull Request #110 · openshift-kni/telco-reference

jmontesi · 2025-02-18T14:33:26Z

The templates cover the reference CRs for the following operators:

LSO
ODF
TALM
ACM
Cluster logging

Also, a make recipe to prevent any mismatch between the templates and the reference CRs is included.

…re tool The templates cover the reference CRs for: * LSO * ODF * TALM * ACM (without the observability configuration) * Cluster logging * Quay The ACM MultiClusterHub is also updated to include the SiteConfig Operator.

openshift-ci · 2025-02-18T14:33:37Z

Hi @jmontesi. Thanks for your PR.

I'm waiting for a openshift-kni member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

imiller0 · 2025-02-18T14:36:42Z

/ok-to-test

imiller0 · 2025-05-27T15:42:12Z

telco-hub/configuration/reference-crs-kube-compare/ReferenceVersionCheck.yaml

+  name: version
+status:
+  desired:
+    version: {{ template "versionMatch" (list .status.desired.version "4.18") }}


Current "main" branch should be 4.19

The reference-crs still point to channel versions related to 4.18. I guess both the reference-crs and the templates should be updated at once.

Main has moved on to 4.20 ;)

There are a couple of changes for 4.19 which are pending based on testing. Until we merge those we are trying to keep main at 4.19 to make things easier.

imiller0 · 2025-05-27T15:44:34Z

telco-hub/configuration/reference-crs-kube-compare/metadata.yaml

+        allOrNoneOf:
+          - path: optional/logging/clusterLogNS.yaml
+          - path: optional/logging/clusterLogOperGroup.yaml
+          - path: optional/logging/clusterLogSubscription.yaml


Should include ClusterLogForwarder here as well.

The ClusterLogForwarder CR is not present in the reference-crs, so I think that CR along with its template should be added in another PR.

Agreed. In reality it is almost entirely customer specific so it can be considered part of their custom overlay and not vetted by the cluster-compare.

imiller0 · 2025-05-27T15:44:58Z

telco-hub/configuration/reference-crs-kube-compare/metadata.yaml

+  - name: optional-quay
+    description: |-
+      TODO: Link to Hub RDS Quay section when published
+    components:
+      - name: quay-operator
+        allOrNoneOf:
+          - path: optional/quay/quayNS.yaml
+          - path: optional/quay/quayOperatorGroup.yaml
+          - path: optional/quay/quaySubscription.yaml


Quay is not yet published in the RDS. Let's omit it here until it is published.

Done. Removed the reference to the Quay operator CR templates.

imiller0 · 2025-05-27T15:46:47Z

telco-hub/configuration/reference-crs-kube-compare/optional/logging/clusterLogNS.yaml

+  annotations:
+    workload.openshift.io/allowed: management


We do not enable workload partitioning for the hub cluster. This annotation should be optional.
One option is we can remove it here entirely and add it to the global ignored list.

imiller0 · 2025-05-27T15:48:55Z

telco-hub/configuration/reference-crs-kube-compare/optional/lso/lsoLocalVolume.yaml

+    {{- range .spec.storageClassDevices }}
+    - storageClassName: {{ .storageClassName }}
+      forceWipeDevicesAndDestroyAllData: true
+      volumeMode: Block


Block or Filesystem is ok.

Done. Both options allowed in the template.

imiller0 · 2025-05-27T15:53:02Z

telco-hub/configuration/reference-crs-kube-compare/optional/quay/quayNS.yaml

@@ -0,0 +1,7 @@
+---


Omit all quay until published.

I have omitted the Quay operator check by removing it from the metadata.yaml. As the source-crs are present in the repo I assume it's also ok to have the templates.

There is a pending PR to remove quay from the repo until we've verified. We should remove it here too.

imiller0 · 2025-05-27T15:54:06Z

telco-hub/configuration/reference-crs-kube-compare/required/acm/acmAgentServiceConfig.yaml

+      - ReadWriteOnce
+    resources:
+      requests:
+        storage: 20Gi


These storage values should check for compliance against a minimum value. (here and below)

I'm not sure how to enforce this condition with the options currently available in the cluster-compare tool. If it was just a number a logical "greater/lower than" could be used but the fact that different capacity units can be used requires more involved processing. Any idea on how to check this?

We don't have any "easy" way to do this, but it may not be too hard to write a helper template to do it. In the short term, it's better to just "allow any value" if this is something customers will be customizing, and open up a new story &PR to restrict the value to a minimum value.

Modified to allow any value.

imiller0 · 2025-05-27T15:54:56Z

telco-hub/configuration/reference-crs-kube-compare/required/acm/acmAgentServiceConfig.yaml

+  # Replace <http-server-address:port> with the address of the local web server that stores the RHCOS images.
+  # The images can be downloaded from "https://mirror.openshift.com/pub/openshift-v4/x86_64/dependencies/rhcos/".
+  - cpuArchitecture: "x86_64"
+    openshiftVersion: "4.17"
+    rootFSUrl: {{ (index .spec.osImages 0).rootFSUrl }}
+    url: {{ (index .spec.osImages 0).url }}
+    version: "417.94.202409121747-0"


We should allow any content here. The specific set of versions they will deploy will vary.

imiller0 · 2025-05-27T15:55:53Z

telco-hub/configuration/reference-crs-kube-compare/required/acm/acmMirrorRegistryCM.yaml

+  labels:
+    app: assisted-service
+data:
+    {{- .data | toYaml | nindent 2 }}


Suggested change

{{- .data | toYaml | nindent 2 }}

{{- .data | toYaml | nindent 2 }}

(technically was not needed because {{- ... eats all whitespace and the later nindent 2 adds the proper carriage return and indentation)

imiller0 · 2025-05-27T15:56:36Z

telco-hub/configuration/reference-crs-kube-compare/required/acm/acmProvisioning.yaml

+  watchAllNamespaces: true
+  # some servers do not support virtual media installations
+  # when the image is served using the https protocol
+  # disableVirtualMediaTLS: true


Make this field optional so that an installation which includes it is ok.

lack · 2025-06-20T17:19:43Z

telco-hub/configuration/reference-crs-kube-compare/metadata.yaml

+      - pathToKey: metadata.ownerReferences
+      - pathToKey: spec.finalizers
+    allowStatusCheck:
+      - include: defatuls


Typo

Suggested change

- include: defatuls

- include: defaults

lack

I added a couple minor coments throughout, but I think one big problem I see is that there's no CI step to ensure that these templates match the actual source-crs in source control. Take a look at how the telco-ran area does this as a good example. Without CI enforcement, you'll get drift as the source-crs and/or cluster-compare templates change over time.

lack · 2025-06-20T17:29:10Z

The other thing I'd recommend before committing this is to run it against a real cluster (or a few real clusters) to make sure you're starting with a fairly clean set of templates that don't have any obvious false-positives to start with.

jmontesi · 2025-06-24T10:23:20Z

I added a couple minor coments throughout, but I think one big problem I see is that there's no CI step to ensure that these templates match the actual source-crs in source control. Take a look at how the telco-ran area does this as a good example. Without CI enforcement, you'll get drift as the source-crs and/or cluster-compare templates change over time.

That's right. I will have a look. In any case I think it's better to open a new PR for this.

jmontesi · 2025-06-24T10:25:28Z

The other thing I'd recommend before committing this is to run it against a real cluster (or a few real clusters) to make sure you're starting with a fairly clean set of templates that don't have any obvious false-positives to start with.

I've tested it against a Hub cluster that I've deployed in the lab. That should provide a fairly clean baseline.

lack · 2025-06-24T13:49:34Z

I added a couple minor coments throughout, but I think one big problem I see is that there's no CI step to ensure that these templates match the actual source-crs in source control. Take a look at how the telco-ran area does this as a good example. Without CI enforcement, you'll get drift as the source-crs and/or cluster-compare templates change over time.

That's right. I will have a look. In any case I think it's better to open a new PR for this.

I understand the reasoning here, but if you don't add enforcement right away, all the other outstanding PRs (#111 #232 #208) will suddenly make these templates out-of-sync.

lack · 2025-07-15T14:40:11Z

/lgtm

imiller0 · 2025-07-16T13:44:31Z

/ok-to-test

jmontesi · 2025-07-17T08:29:23Z

/retest

imiller0 · 2025-07-17T13:12:36Z

/ok-to-test

lack

Looking good! My only concern is that we should not duplicate the CI scripts.

lack · 2025-07-17T13:19:40Z

telco-hub/configuration/reference-crs-kube-compare/Makefile

Now that we have 2 areas that do the same check (here and telco-ran), maybe we should consider putting more of the logic in a shared helper script instead of copying it?

To be implemented in a follow-up PR as discussed.

lack · 2025-07-17T13:21:56Z

telco-hub/configuration/reference-crs-kube-compare/compare.sh

Likewise, let's share the same compare.sh with telco-ran as opposed to making a copy.

To be implemented in a follow-up PR as discussed.

lack · 2025-07-17T13:22:44Z

telco-hub/configuration/reference-crs-kube-compare/ReferenceVersionCheck.yaml

+  name: version
+status:
+  desired:
+    version: {{ template "versionMatch" (list .status.desired.version "4.19") }}


HEAD is currently 4.20, isn't it?

Currently 4.19 which will very soon get copied back to release-4.19 and then main updated to 4.20

lack · 2025-07-17T13:23:54Z

telco-hub/configuration/reference-crs-kube-compare/metadata.yaml

+  - name: version-check
+    description: |-
+      A mismatch here means you may be using the wrong reference.
+      This reference was designed for OpenShift 4.18.


Suggested change

This reference was designed for OpenShift 4.18.

This reference was designed for OpenShift 4.20

imiller0 · 2025-07-22T14:58:15Z

/ok-to-test

imiller0

Two minor comments, otherwise LGTM

imiller0 · 2025-07-23T18:30:30Z

telco-hub/configuration/reference-crs-kube-compare/Makefile

+
+.PHONY: sync
+sync: convert
+	@./compare.sh --sync "../reference-crs" renderedv1 


Fix newline please

imiller0 · 2025-07-23T18:30:51Z

telco-hub/configuration/reference-crs-kube-compare/compare.sh

+    sync_cr "$RENDERDIR" "$SOURCEDIR" compare_ignore
+else
+    compare_cr "$RENDERDIR" "$SOURCEDIR" compare_ignore
+fi 


Fix newline please

lack · 2025-07-25T19:06:04Z

/ok-to-test

imiller0

/lgtm

imiller0 · 2025-08-04T20:01:00Z

/ok-to-test

imiller0

/lgtm

openshift-ci · 2025-08-05T17:57:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: imiller0, jmontesi

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~~ [imiller0]

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

imiller0 · 2025-08-05T17:58:03Z

/ok-to-test

jmontesi added 2 commits February 18, 2025 11:59

Skip yaml linting for the template CRs

05ef418

openshift-ci bot requested review from MarSik and lack February 18, 2025 14:33

openshift-ci bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Feb 18, 2025

openshift-ci bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 18, 2025

jmontesi added 5 commits May 12, 2025 11:18

Merge branch 'main' into telco-hub-kube-compare

260dd53

Merge branch 'main' into telco-hub-kube-compare

c692667

Merge branch 'main' into telco-hub-kube-compare

6a4eba5

Update templates to OCP 4.18 and handle ArgoCD metadata

1fb009a

Update lint checking

097ca3b

imiller0 reviewed May 27, 2025

View reviewed changes

jmontesi added 4 commits May 29, 2025 12:18

Merge branch 'main' into telco-hub-kube-compare

a717a8a

Minor changes to address review comments

3eec9ff

Merge branch 'main' into telco-hub-kube-compare

47e6167

Add observability config templates and links to the published RDS doc

ae21130

lack reviewed Jun 20, 2025

View reviewed changes

lack suggested changes Jun 20, 2025

View reviewed changes

openshift-ci bot assigned lack Jun 20, 2025

Addressed review comments

987381a

jmontesi added 3 commits June 26, 2025 11:21

Merge branch 'main' into telco-hub-kube-compare

3bbe1b7

Split the ACM pull secret handling CRs into different files

1b34ced

Add a make recipe to check the aligment of reference CRs and templates

93f16d7

jmontesi added 2 commits July 15, 2025 14:47

Merge branch 'main' into telco-hub-kube-compare

2da5e68

Changes to align with main branch update to 4.19

094ec1d

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 15, 2025

Merge branch 'main' into telco-hub-kube-compare

0a8f11c

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jul 17, 2025

lack suggested changes Jul 17, 2025

View reviewed changes

jmontesi added 2 commits July 20, 2025 17:52

Avoid modifying the reference CRs as much as possible

dbf34cd

Merge branch 'main' into telco-hub-kube-compare

43206ac

imiller0 reviewed Jul 23, 2025

View reviewed changes

Fix newline

c15a291

imiller0 approved these changes Aug 4, 2025

View reviewed changes

openshift-ci bot assigned imiller0 Aug 4, 2025

openshift-ci bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 4, 2025

jmontesi added 3 commits August 5, 2025 09:34

Merge branch 'main' into telco-hub-kube-compare

725422b

Disable cluster-proxy-addon in the acmMCE cluster-compare template

b064ed7

Rebase changes and CI fixes

155be99

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2025

imiller0 approved these changes Aug 5, 2025

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 5, 2025

openshift-merge-bot bot merged commit d14dcfb into openshift-kni:main Aug 5, 2025
5 checks passed

	{{- .data \| toYaml \| nindent 2 }}
	{{- .data \| toYaml \| nindent 2 }}

	This reference was designed for OpenShift 4.18.
	This reference was designed for OpenShift 4.20

Comments

Conversation

jmontesi commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Feb 18, 2025

Uh oh!

imiller0 commented Feb 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lack left a comment

Choose a reason for hiding this comment

Uh oh!

lack commented Jun 20, 2025

Uh oh!

jmontesi commented Jun 24, 2025

Uh oh!

jmontesi commented Jun 24, 2025

Uh oh!

lack commented Jun 24, 2025

Uh oh!

lack commented Jul 15, 2025

Uh oh!

jmontesi commented Feb 18, 2025 •

edited

Loading

jmontesi Jul 20, 2025 •

edited

Loading