CNF-22194 RAN: ensure one copy of MachineConfigs in RAN RDS - #826
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesInstall directory layout and documentation
Sequence Diagram(s)sequenceDiagram
participant RootMakefile
participant InstallMakefile
participant ConfigMakefile
participant CompareSh
participant BuilderMakefile
RootMakefile->>InstallMakefile: make check in ./telco-ran/install
InstallMakefile->>BuilderMakefile: make -C ./extra-manifests-builder check
ConfigMakefile->>CompareSh: ./compare.sh --check-extra-manifests
CompareSh-->>ConfigMakefile: exit status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
openshift-ci
Bot
added
the
needs-ok-to-test
|
Hi @kchennag. 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 Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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. |
| profile: ran-du-performance | ||
| - path: source-crs/optional-extra-manifest/enable-crun-master.yaml | ||
| - path: source-crs/optional-extra-manifest/enable-crun-worker.yaml | ||
| profile: ran-du-performance No newline at end of file |
There was a problem hiding this comment.
Please remove this new line (and the other occurrences below)
There was a problem hiding this comment.
fixed, removed trailing whitespace and added missing EOF newlines on all affected ACM PolicyGenerator files.
There was a problem hiding this comment.
Do we still need this makefile under telco-ran/configuration?
There was a problem hiding this comment.
yes it still renders the reference MachineConfig YAML into telco-ran/install/extra-manifests/. only the output path (DST) changed, the builder stays under configuration/ because it is part of the day-2/CI tooling tree, not install artifacts.
There was a problem hiding this comment.
Thanks! I think we can use this opportunity and move this extra-manifests-builder folder out of here and into telco-ran/install as well.
Lets create a Makefile under telco-ran/install, move the checkExtraManifests target to this new Makefile and link it in the root Makefile target here check-reference-ran. Prow targets this, so functionality should remain same as before.
| @@ -1,3 +1,4 @@ | |||
| # Retained on release-4.21. Remove in main (4.23+) only; do not backport removal. | |||
There was a problem hiding this comment.
Can we remove this comment?
There was a problem hiding this comment.
done, removed the release comment
| @@ -1,3 +1,4 @@ | |||
| # Retained on release-4.21. Remove in main (4.23+) only; do not backport removal. | |||
There was a problem hiding this comment.
Same, do we need this comment?
There was a problem hiding this comment.
done removed from enable-crun-worker.yaml in kube-compare and custom-manifests as well
| - ./extra-manifest contains the MachineConfig CRs to be applied during the cluster installation | ||
| - ./validatorCRs contains the validation CRs to be deployed via the ACM policies to validate some configuration | ||
|
|
||
| Install-time MachineConfig CRs live under `../../install/extra-manifests/` (see `../../install/README.md`). |
There was a problem hiding this comment.
Maybe a better place for this would be a readme under telco-ran/install? we dont need this in the telco-ran/configuration readme meant for day-2 configuration
There was a problem hiding this comment.
agreed removed the install-time note from source-crs/README.md install documentation is in telco-ran/install/README.md only
| @@ -0,0 +1,32 @@ | |||
| --- | |||
| # Example: build install-time extra-manifests ConfigMap for ClusterInstance. | |||
| # Apply with: kubectl apply -k telco-ran/install/ | |||
There was a problem hiding this comment.
This comment here is inaccurate, we should not dictate customers to kubectl apply the manifests. Please move the example ClusterInstance files and include a kustomization like this to avoid confusion.
https://github.com/openshift-kni/telco-reference/blob/main/telco-ran/configuration/argocd/example/clusterinstance/kustomization.yaml
There was a problem hiding this comment.
done removed top-level install/kustomization.yaml and all kubectl apply guidance clusterInstance examples are under telco-ran/install/clusterinstance/ with resources + configMapGenerator pointing at ../extra-manifests/, following the existing example pattern
| - `extra-manifests/` — reference MachineConfig and related CRs applied at cluster install | ||
| - `custom-manifests/` — optional manifests (for example `enable-crun-*.yaml`) you add when needed | ||
| - `kustomization.yaml` — example `configMapGenerator` for `ClusterInstance.spec.extraManifestsRefs` | ||
| - `clusterinstance` — symlink to `../configuration/argocd/example/clusterinstance/` |
There was a problem hiding this comment.
Could we move the clusterinstance example folder under telco-ran/install and remove this symlink
There was a problem hiding this comment.
done, moved clusterinstance/ examples to telco-ran/install/clusterinstance/ and removed the symlink
| ## ClusterInstance and extra-manifests ConfigMap | ||
|
|
||
| ```bash | ||
| kubectl apply -k telco-ran/install/ |
There was a problem hiding this comment.
Please remove this. The recommended method of using these CRs is through gitops, we should not declare this here.
There was a problem hiding this comment.
done removed the kubectl apply section from install/README.md; it now documents layout and GitOps/day-N policy only. Also added a PR summary with changes, link to Core #825, and note on the cnf-features-deploy Containerfile follow-up
|
Please also add a short description on the PR regarding the changes, thanks! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
telco-ran/install/clusterinstance/kustomization.yaml (1)
10-33: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winWire
custom-manifests/into the install bundle.
telco-ran/install/custom-manifests/README.mdsays optional CRUN manifests should live behind a separateClusterInstance.spec.extraManifestsRefsConfigMap, but this kustomization only emits the basesno-extra-manifest-configmapfrom../extra-manifests/. As written, there is no install-time ConfigMap for the new optional directory, so the documented CRUN flow cannot actually be referenced. Add a second generator for../custom-manifests/(or tighten the README) so the layout matches the shipped artifacts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@telco-ran/install/clusterinstance/kustomization.yaml` around lines 10 - 33, Add a second configMapGenerator block in the configMapGenerator list in the kustomization.yaml file to process files from the ../custom-manifests/ directory. This new generator should mirror the structure of the existing sno-extra-manifest-configmap generator (which processes ../extra-manifests/) but point to ../custom-manifests/ instead and be named appropriately (such as sno-custom-manifest-configmap) with the same namespace reference to enable the documented optional CRUN manifests flow. Place this new generator block immediately after the existing extra-manifests generator and before the generatorOptions section.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@telco-ran/configuration/argocd/AdditionalManifests.md`:
- Around line 9-35: The documentation example currently references the old
kustomization file path telco-ran/install/kustomization.yaml. Update the
reference to point to the new location
telco-ran/install/clusterinstance/kustomization.yaml so readers access the
correct file with the configMapGenerator configuration.
In `@telco-ran/configuration/source-crs/optional-extra-manifest/README.md`:
- Around line 3-4: The relative path `../../install/custom-manifests/` in the
README.md file located at
telco-ran/configuration/source-crs/optional-extra-manifest/README.md is
incorrect and only goes up two directory levels, causing it to resolve to
configuration/install/ instead of the repo-root install/ directory. Change the
relative path from `../../install/custom-manifests/` to
`../../../install/custom-manifests/` to correctly reference the target location
at the repository root level.
---
Outside diff comments:
In `@telco-ran/install/clusterinstance/kustomization.yaml`:
- Around line 10-33: Add a second configMapGenerator block in the
configMapGenerator list in the kustomization.yaml file to process files from the
../custom-manifests/ directory. This new generator should mirror the structure
of the existing sno-extra-manifest-configmap generator (which processes
../extra-manifests/) but point to ../custom-manifests/ instead and be named
appropriately (such as sno-custom-manifest-configmap) with the same namespace
reference to enable the documented optional CRUN manifests flow. Place this new
generator block immediately after the existing extra-manifests generator and
before the generatorOptions section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: aa0e6cfc-22d9-45db-b0cd-4fac5af2f32f
📒 Files selected for processing (56)
VERSION_UPDATE_GUIDE.mdhack/test-kustomize.shtelco-ran/configuration/Makefiletelco-ran/configuration/README.mdtelco-ran/configuration/argocd/AdditionalManifests.mdtelco-ran/configuration/argocd/SNOExpansion.mdtelco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-3node-ranGen.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-sno-ranGen.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-standard-ranGen.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-3node-ranGen-templated.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-sno-ranGen-templated.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-standard-ranGen-templated.yamltelco-ran/configuration/argocd/example/clusterinstance/kustomization.yamltelco-ran/configuration/argocd/example/policygentemplates/group-du-3node-ranGen.yamltelco-ran/configuration/argocd/example/policygentemplates/group-du-sno-ranGen.yamltelco-ran/configuration/argocd/example/policygentemplates/group-du-standard-ranGen.yamltelco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-3node-ranGen-templated.yamltelco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-sno-ranGen-templated.yamltelco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-standard-ranGen-templated.yamltelco-ran/configuration/compare.shtelco-ran/configuration/extra-manifests-builder/Makefiletelco-ran/configuration/extra-manifests-builder/README.mdtelco-ran/configuration/kube-compare-reference/hack/compare_ignoretelco-ran/configuration/source-crs/README.mdtelco-ran/configuration/source-crs/deprecated/MachineConfigContainerMountNS.yamltelco-ran/configuration/source-crs/machine-config/MachineConfigSctp.yamltelco-ran/configuration/source-crs/optional-extra-manifest/README.mdtelco-ran/install/README.mdtelco-ran/install/clusterinstancetelco-ran/install/clusterinstance/README.mdtelco-ran/install/clusterinstance/example-3node.yamltelco-ran/install/clusterinstance/example-sno.yamltelco-ran/install/clusterinstance/example-standard.yamltelco-ran/install/clusterinstance/kustomization.yamltelco-ran/install/custom-manifests/README.mdtelco-ran/install/custom-manifests/enable-crun-master.yamltelco-ran/install/custom-manifests/enable-crun-worker.yamltelco-ran/install/extra-manifests/01-container-mount-ns-and-kubelet-conf-master.yamltelco-ran/install/extra-manifests/01-container-mount-ns-and-kubelet-conf-worker.yamltelco-ran/install/extra-manifests/01-disk-encryption-pcr-rebind-master.yamltelco-ran/install/extra-manifests/01-disk-encryption-pcr-rebind-worker.yamltelco-ran/install/extra-manifests/03-sctp-machine-config-master.yamltelco-ran/install/extra-manifests/03-sctp-machine-config-worker.yamltelco-ran/install/extra-manifests/06-kdump-master.yamltelco-ran/install/extra-manifests/06-kdump-worker.yamltelco-ran/install/extra-manifests/07-sriov-related-kernel-args-master.yamltelco-ran/install/extra-manifests/07-sriov-related-kernel-args-worker.yamltelco-ran/install/extra-manifests/08-set-rcu-normal-master.yamltelco-ran/install/extra-manifests/08-set-rcu-normal-worker.yamltelco-ran/install/extra-manifests/09-openshift-marketplace-ns.yamltelco-ran/install/extra-manifests/10-rename-gnrd-interfaces-master.yamltelco-ran/install/extra-manifests/10-rename-gnrd-interfaces-worker.yamltelco-ran/install/extra-manifests/99-sync-time-once-master.yamltelco-ran/install/extra-manifests/99-sync-time-once-worker.yamltelco-ran/install/extra-manifests/disk-encryption.yaml.tmpltelco-ran/install/extra-manifests/image-registry-partition-mc.yaml.tmpl
💤 Files with no reviewable changes (11)
- telco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-3node-ranGen-templated.yaml
- telco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-sno-ranGen.yaml
- telco-ran/configuration/kube-compare-reference/hack/compare_ignore
- telco-ran/configuration/source-crs/README.md
- telco-ran/install/clusterinstance
- telco-ran/configuration/argocd/example/clusterinstance/kustomization.yaml
- telco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-3node-ranGen.yaml
- telco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-standard-ranGen-templated.yaml
- telco-ran/configuration/argocd/example/policygentemplates/group-du-3node-ranGen.yaml
- telco-ran/configuration/argocd/example/policygentemplates/group-du-sno-ranGen.yaml
- telco-ran/configuration/argocd/example/policygentemplates/group-du-standard-ranGen.yaml
There was a problem hiding this comment.
Thanks! I think we can use this opportunity and move this extra-manifests-builder folder out of here and into telco-ran/install as well.
Lets create a Makefile under telco-ran/install, move the checkExtraManifests target to this new Makefile and link it in the root Makefile target here check-reference-ran. Prow targets this, so functionality should remain same as before.
| @@ -1 +1 @@ | |||
| ../extra-manifest/03-sctp-machine-config-master.yaml No newline at end of file | |||
| ../../../install/extra-manifests/03-sctp-machine-config-master.yaml No newline at end of file | |||
There was a problem hiding this comment.
nit: missing newline in this file
There was a problem hiding this comment.
Please fix in all locations
|
|
||
| configMapGenerator: | ||
| - files: | ||
| - ../extra-manifests/01-container-mount-ns-and-kubelet-conf-master.yaml |
There was a problem hiding this comment.
Hmmm I think this path will not resolve correctly. Kustomize cannot look beyond its root folder by default, so (../) path would break in an argo environment. One way to test would be to kustomize apply on a dev k8s environment and see if the resources are rendered.
For reference,
https://kubernetes.io/docs/reference/kubectl/generated/kubectl_kustomize/
check for docs under load-restrictor arg, this is not set by default in an Argo setup.
|
/ok-to-test |
openshift-ci
Bot
added
ok-to-test
8cbfcca to
fc26038
Compare
openshift-ci
Bot
removed
the
needs-rebase
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
118-120: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWire
test-shellcheckinto a CI entrypoint.This target is added, but the
ci-validateflow shown below still does not invoke it, so the new shellcheck dependency does not actually protect the changed shell scripts. Please addtest-shellchecktoci-validateor another always-run validation target.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 118 - 120, The new shellcheck validation target is defined in the Makefile but is not yet part of the always-run CI validation flow. Update ci-validate (or the equivalent top-level validation target it invokes) to include test-shellcheck so the shell script checks run automatically alongside the other validations. Use the existing target names ci-validate and test-shellcheck to locate the integration point and keep the new check wired into CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@telco-ran/install/extra-manifests/10-rename-gnrd-interfaces-master.yaml`:
- Line 25: Remove the extra generated blank line causing the YAMLlint failure by
fixing the source template in extra-manifests-builder/template rather than
editing only the generated manifest output. Locate the blank-line generation in
the template used to produce 10-rename-gnrd-interfaces-master.yaml and adjust
the rendering logic so future regenerations do not reintroduce the invalid empty
line.
---
Nitpick comments:
In `@Makefile`:
- Around line 118-120: The new shellcheck validation target is defined in the
Makefile but is not yet part of the always-run CI validation flow. Update
ci-validate (or the equivalent top-level validation target it invokes) to
include test-shellcheck so the shell script checks run automatically alongside
the other validations. Use the existing target names ci-validate and
test-shellcheck to locate the integration point and keep the new check wired
into CI.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: d65f6a7a-e9b4-4445-bfab-45befc3c6bd8
📒 Files selected for processing (93)
MakefileVERSION_UPDATE_GUIDE.mdhack/test-kustomize.shtelco-ran/configuration/Makefiletelco-ran/configuration/README.mdtelco-ran/configuration/argocd/AdditionalManifests.mdtelco-ran/configuration/argocd/SNOExpansion.mdtelco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-3node-ranGen.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-sno-ranGen.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-standard-ranGen.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-3node-ranGen-templated.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-sno-ranGen-templated.yamltelco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-standard-ranGen-templated.yamltelco-ran/configuration/argocd/example/clusterinstance/kustomization.yamltelco-ran/configuration/argocd/example/policygentemplates/group-du-3node-ranGen.yamltelco-ran/configuration/argocd/example/policygentemplates/group-du-sno-ranGen.yamltelco-ran/configuration/argocd/example/policygentemplates/group-du-standard-ranGen.yamltelco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-3node-ranGen-templated.yamltelco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-sno-ranGen-templated.yamltelco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-standard-ranGen-templated.yamltelco-ran/configuration/compare.shtelco-ran/configuration/kube-compare-reference/hack/compare_ignoretelco-ran/configuration/source-crs/README.mdtelco-ran/configuration/source-crs/deprecated/MachineConfigContainerMountNS.yamltelco-ran/configuration/source-crs/machine-config/MachineConfigSctp.yamltelco-ran/configuration/source-crs/optional-extra-manifest/README.mdtelco-ran/install/Makefiletelco-ran/install/README.mdtelco-ran/install/clusterinstancetelco-ran/install/clusterinstance/README.mdtelco-ran/install/clusterinstance/example-3node.yamltelco-ran/install/clusterinstance/example-sno.yamltelco-ran/install/clusterinstance/example-standard.yamltelco-ran/install/clusterinstance/kustomization.yamltelco-ran/install/custom-manifests/README.mdtelco-ran/install/custom-manifests/enable-crun-master.yamltelco-ran/install/custom-manifests/enable-crun-worker.yamltelco-ran/install/extra-manifests-builder/.gitignoretelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/30-kubelet-interval-tuning.conftelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/90-container-mount-namespace-kubelet.conftelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/90-container-mount-namespace.conftelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/build.shtelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/container-mount-namespace.servicetelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/extractExecStarttelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/nsenterCmnstelco-ran/install/extra-manifests-builder/01-container-mount-ns-and-kubelet-conf/test.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/README.mdtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/build.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/disablePcrOnRebootOrShutdown.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/efibootmgr-output.txttelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/hwupgrade-detection-methods/file.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/hwupgrade-detection-methods/fwup.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/hwupgrade-detection-methods/ostree.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/hwupgrade-detection-methods/talm.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/luks-helpers.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/order.conftelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/pcr-disable-shutdown.servicetelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/pcr-rebind-boot.servicetelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/rebindDiskOnBoot.shtelco-ran/install/extra-manifests-builder/01-disk-encryption-pcr-rebind/test.shtelco-ran/install/extra-manifests-builder/03-sctp-machine-config/build.shtelco-ran/install/extra-manifests-builder/06-kdump/build.shtelco-ran/install/extra-manifests-builder/07-sriov-related-kernel-args/build.shtelco-ran/install/extra-manifests-builder/08-set-rcu-normal/README.mdtelco-ran/install/extra-manifests-builder/08-set-rcu-normal/build.shtelco-ran/install/extra-manifests-builder/08-set-rcu-normal/set-rcu-normal.servicetelco-ran/install/extra-manifests-builder/08-set-rcu-normal/set-rcu-normal.shtelco-ran/install/extra-manifests-builder/08-set-rcu-normal/test.shtelco-ran/install/extra-manifests-builder/10-rename-gnrd-interfaces/build.shtelco-ran/install/extra-manifests-builder/99-sync-time-once/build.shtelco-ran/install/extra-manifests-builder/Makefiletelco-ran/install/extra-manifests-builder/README.mdtelco-ran/install/extra-manifests-builder/tools.gotelco-ran/install/extra-manifests/01-container-mount-ns-and-kubelet-conf-master.yamltelco-ran/install/extra-manifests/01-container-mount-ns-and-kubelet-conf-worker.yamltelco-ran/install/extra-manifests/01-disk-encryption-pcr-rebind-master.yamltelco-ran/install/extra-manifests/01-disk-encryption-pcr-rebind-worker.yamltelco-ran/install/extra-manifests/03-sctp-machine-config-master.yamltelco-ran/install/extra-manifests/03-sctp-machine-config-worker.yamltelco-ran/install/extra-manifests/06-kdump-master.yamltelco-ran/install/extra-manifests/06-kdump-worker.yamltelco-ran/install/extra-manifests/07-sriov-related-kernel-args-master.yamltelco-ran/install/extra-manifests/07-sriov-related-kernel-args-worker.yamltelco-ran/install/extra-manifests/08-set-rcu-normal-master.yamltelco-ran/install/extra-manifests/08-set-rcu-normal-worker.yamltelco-ran/install/extra-manifests/09-openshift-marketplace-ns.yamltelco-ran/install/extra-manifests/10-rename-gnrd-interfaces-master.yamltelco-ran/install/extra-manifests/10-rename-gnrd-interfaces-worker.yamltelco-ran/install/extra-manifests/99-sync-time-once-master.yamltelco-ran/install/extra-manifests/99-sync-time-once-worker.yamltelco-ran/install/extra-manifests/disk-encryption.yaml.tmpltelco-ran/install/extra-manifests/image-registry-partition-mc.yaml.tmpltelco-ran/install/kustomization.yaml
💤 Files with no reviewable changes (9)
- telco-ran/configuration/source-crs/README.md
- telco-ran/configuration/argocd/example/policygentemplates/group-du-3node-ranGen.yaml
- telco-ran/install/clusterinstance
- telco-ran/configuration/kube-compare-reference/hack/compare_ignore
- telco-ran/configuration/argocd/example/clusterinstance/kustomization.yaml
- telco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-3node-ranGen-templated.yaml
- telco-ran/configuration/argocd/example/policygentemplates/group-du-standard-ranGen.yaml
- hack/test-kustomize.sh
- telco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-standard-ranGen-templated.yaml
✅ Files skipped from review due to trivial changes (13)
- telco-ran/configuration/source-crs/optional-extra-manifest/README.md
- telco-ran/configuration/source-crs/deprecated/MachineConfigContainerMountNS.yaml
- telco-ran/install/custom-manifests/README.md
- telco-ran/configuration/argocd/example/policygentemplates/group-du-sno-ranGen.yaml
- VERSION_UPDATE_GUIDE.md
- telco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-3node-ranGen-templated.yaml
- telco-ran/configuration/source-crs/machine-config/MachineConfigSctp.yaml
- telco-ran/install/extra-manifests-builder/README.md
- telco-ran/configuration/argocd/SNOExpansion.md
- telco-ran/configuration/argocd/AdditionalManifests.md
- telco-ran/configuration/argocd/example/acmpolicygenerator/hub-side-templating/acm-group-du-standard-ranGen-templated.yaml
- telco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-standard-ranGen.yaml
- telco-ran/install/README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- telco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-sno-ranGen.yaml
- telco-ran/configuration/README.md
- telco-ran/configuration/argocd/example/acmpolicygenerator/acm-group-du-3node-ranGen.yaml
- telco-ran/configuration/argocd/example/policygentemplates/hub-side-templating/group-du-sno-ranGen-templated.yaml
- telco-ran/configuration/compare.sh
| overwrite: true | ||
| contents: | ||
| source: data:text/plain,%5BMatch%5D%0AProperty%3DID_VENDOR_ID%3D0x8086%0AProperty%3DID_MODEL_ID%3D0x12d3%0A%0A%5BLink%5D%0ANamePolicy%3Dpath%0A | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the extra generated blank line.
This artifact now violates YAMLlint, so the PR is carrying a check-breaking manifest. Since the file is generated, please fix the extra-manifests-builder/template rather than only trimming this output, otherwise the next regeneration will reintroduce it.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 25-25: too many blank lines (1 > 0)
(empty-lines)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@telco-ran/install/extra-manifests/10-rename-gnrd-interfaces-master.yaml` at
line 25, Remove the extra generated blank line causing the YAMLlint failure by
fixing the source template in extra-manifests-builder/template rather than
editing only the generated manifest output. Locate the blank-line generation in
the template used to produce 10-rename-gnrd-interfaces-master.yaml and adjust
the rendering logic so future regenerations do not reintroduce the invalid empty
line.
Source: Linters/SAST tools
|
/lgtm |
|
Thanks for the changes! |
|
Hi @abraham2512, I've opened the cnf-features-deploy PR to update the ZTP Containerfile for the folder restructuring: openshift-kni/cnf-features-deploy#4333 |
6c993dd to
6c562a0
Compare
| @@ -1 +1 @@ | |||
| ../extra-manifest/03-sctp-machine-config-master.yaml No newline at end of file | |||
| ../../../install/extra-manifests/03-sctp-machine-config-master.yaml No newline at end of file | |||
There was a problem hiding this comment.
Please fix in all locations
| - example-3node.yaml | ||
| - example-standard.yaml | ||
| # - namespace.yaml | ||
| # - secret.yaml |
There was a problem hiding this comment.
Include example of building the necessary confgmap with all extra manifests -- needed for ClusterInstance install flow and must be usable by the policy which ensures the extra manifests are covered by Policy at day-n
There was a problem hiding this comment.
@kchennag please update the builder to render extra-manifests inside telco-ran/install/clusterinstance. This way you can reference them in the parent kustomize configmap generator
|
/retest |
03ab741 to
6a26a09
Compare
|
/ok-to-test |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abraham2512, irinamihai, kchennag The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
Bot
added
the
approved
|
/retest |
5 participants
CNF-22194 Telco RAN install manifest deduplication-