-
Notifications
You must be signed in to change notification settings - Fork 1.8k
TELCODOCS-1476 hub-side templating #68139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🤖 Updated build preview is available at: Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/35226 |
5aaaecd
to
b1216dc
Compare
[id="ztp-deploying-siteconfig-hub-side-template_{context}"] | ||
= Deploying a SiteConfig with hub-side templating | ||
|
||
The following is an example `SiteConfig` custom resource (CR) that uses the `spec.clusters.siteConfigMap.data` field to specify per-site data to be used in hub side templating. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following is an example `SiteConfig` custom resource (CR) that uses the `spec.clusters.siteConfigMap.data` field to specify per-site data to be used in hub side templating. | |
The following is a `SiteConfig` custom resource (CR) example that includes the new optional `spec.clusters.siteConfigMap` where the `data` field contains per-site data to be used in hub side templating. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There can be an issue with referring to a feature as "new" or "optional" in product documentation because, as time passes, the feature ceases to be new but the documentation won't be updated to reflect that. Release Notes are the best place to point out the newness of a feature.
I've rephrased though, in order to draw attention to the spec.clusters.siteConfigMap
section and mention the data
field separately
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, but I think we should still say that it's an optional section, it's not mandatory. 🤔
---- | ||
[NOTE] | ||
==== | ||
The generated `ConfigMap` CRs follow the naming convention `ztp-site-<cluster_name>-configMap` and are created under the same namespace as the policy resource. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just leave
The generated
ConfigMap
CRs follow the naming conventionztp-site-<cluster_name>
.
since you're explaining the namespace aspect below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. I removed reference to the namespace and left it at ztp-site-<cluster_name>-configMap
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be ztp-site-<cluster-name>
though.
==== | ||
The generated `ConfigMap` CRs follow the naming convention `ztp-site-<cluster_name>-configMap` and are created under the same namespace as the policy resource. | ||
The `siteConfigMap` section includes the namespace where the `ConfigMap` is to be created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `siteConfigMap` section includes the namespace where the `ConfigMap` is to be created. | |
The `siteConfigMap` section includes the namespace where the `ConfigMap` is to be created and its name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry @irinamihai I'm not clear on what the "its" refers to here.
Is "its name" the name of the namespace or the name of the ConfigMap
?
The "name of the ConfigMap" doesn't seem right but the "name of the namespace" sounds kinda off too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name refers to ConfigMap - add this to the example yaml
Something like
SiteConfigMap
name: site-example-sno
The generated `ConfigMap` CRs follow the naming convention `ztp-site-<cluster_name>-configMap` and are created under the same namespace as the policy resource. | ||
The `siteConfigMap` section includes the namespace where the `ConfigMap` is to be created. | ||
The `siteConfigMap.namespace` you specify must be the same as the namespace used by the `PolicyGenTemplate` resources. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The `siteConfigMap.namespace` you specify must be the same as the namespace used by the `PolicyGenTemplate` resources. | |
For a successful integration with the PolicyGenTemplate, the `siteConfigMap.namespace` you specify must be the same as the namespace used by the `PolicyGenTemplate` resources. |
---- | ||
include::snippets/ztp_example-siteconfig-hub-side-templating.yaml[] | ||
---- | ||
[NOTE] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the section below can be a bit restructured to something like:
Besides the
data
field, thesiteConfigMap
section also includes the namespace where theConfigMap
is to be created and its name.
For a successful integration with the
PolicyGenTemplate
, thesiteConfigMap.namespace
you specify must be the same as the namespace used by thePolicyGenTemplate
resources.
If not specified,
siteConfigMap.namespace
will default to , andsiteConfigMap.namespace
to ztp-site-<cluster_name>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could have another look at this together?
..... | ||
---- | ||
|
||
The following example uses hub-side templating to generate multiple `machineConfigs` from a single {rh-rhacm} policy: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following example uses hub side templating to generate PerformanceProfile
{rh-rhacm} policies for the cluster that have the following labels: group-du-sno-zone: "zone-1"
, hardware-type: "dell-poweredge-xr12"
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I've proposed a change along these lines.
b1216dc
to
956b437
Compare
956b437
to
63a28e4
Compare
modules/ztp-deploying-a-site.adoc
Outdated
dell-poweredge-xr12-ptpcfgslave-profile-interface: "ens5f0" | ||
dell-poweredge-xr12-cpu-isolated: "2-19,22-39" | ||
dell-poweredge-xr12-cpu-reserved: "0-1,20-21" | ||
dell-poweredge-xr12-hugepages-default: "1G" | ||
dell-poweredge-xr12-hugepages-size: "1G" | ||
dell-poweredge-xr12-hugepages-count: "32" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can remove this and instead include:
sno-1-sriov-network-vlan-1: "140"
sno-1-sriov-network-vlan-2: "150"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
modules/ztp-deploying-a-site.adoc
Outdated
+ | ||
[NOTE] | ||
==== | ||
As an alternative to populating the siteConfigMap section in the `SiteConfig` CR you can store site-specific template data in the ZTP GitOps repo, as `ConfigMap` CRs, then push this template data to the hub cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an alternative to populating the siteConfigMap section in the `SiteConfig` CR you can store site-specific template data in the ZTP GitOps repo, as `ConfigMap` CRs, then push this template data to the hub cluster. | |
As an alternative to populating the siteConfigMap section in the `SiteConfig` CR you can store site-specific template data in the ZTP GitOps repo, as `ConfigMap` CRs, and have them synced to the hub cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
include::snippets/ztp_example-pgt-hub-side-templating.yaml[] | ||
---- | ||
|
||
Using the `fromConfigMap` function, the file defines site-specific configurations for each cluster defined in the `PolicyGenTemplate`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the `fromConfigMap` function, the file defines site-specific configurations for each cluster defined in the `PolicyGenTemplate`. | |
Using the `fromConfigMap` function, site-specific configuration is selected for each cluster defined in the `PolicyGenTemplate`. |
---- | ||
|
||
Using the `fromConfigMap` function, the file defines site-specific configurations for each cluster defined in the `PolicyGenTemplate`. | ||
This example generates policies for clsuters that have the labels: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example generates policies for clsuters that have the labels: | |
This example generates policies for clusters that have the labels: |
21d709c
to
81a7e64
Compare
81a7e64
to
78ef28a
Compare
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
PR needs rebase. 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/test-infra repository. |
@slovern: The following test failed, say
Full PR test history. Your PR dashboard. 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/test-infra repository. I understand the commands that are listed here. |
Stale issues rot after 30d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle rotten |
Rotten issues close after 30d of inactivity. Reopen the issue by commenting /close |
@openshift-bot: Closed this PR. In response to this:
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. |
ON HOLD subject to updates to TALM (https://issues.redhat.com/browse/CNF-9102)
Version(s): TBD
Issue: https://issues.redhat.com/browse/TELCODOCS-1476
Link to docs preview:
About the PolicyGenTemplate CRD (new example PGT added)
Deploying a managed cluster with SiteConfig and GitOps ZTP
(added new optional procedure step 3.e)
QE review:
Additional information: