Skip to content
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

TELCODOCS-597: Adding a local registry to a Single Node OpenShift DU profile #50904

Merged
merged 1 commit into from Dec 5, 2022

Conversation

tmulquee
Copy link
Contributor

@tmulquee tmulquee commented Sep 27, 2022

@openshift-ci openshift-ci bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 27, 2022
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Sep 27, 2022

🤖 Updated build preview is available at:
https://50904--docspreview.netlify.app

Build log: https://circleci.com/gh/ocpdocs-previewbot/openshift-docs/4452

@tmulquee tmulquee force-pushed the TELCODOCS-597 branch 2 times, most recently from 7f7e51d to 68a19ef Compare October 12, 2022 15:00
@tmulquee tmulquee changed the title TELCODOCS-597: TELCODOCS-597: Adding a local registry to a Single Node OpenShift DU profile Oct 13, 2022
@tmulquee tmulquee force-pushed the TELCODOCS-597 branch 2 times, most recently from 444a494 to 00c41bd Compare October 13, 2022 13:44

This is unavoidable for the initial deployment. Over time, there is a risk that `crio` will wipe `/var/lib/containers/storage` in the case of a disorderly shutdown.

To address this, you need a local registry on the SNO. The cluster images need to be mirrored to this registry along with being used as the registry for the applications.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's important to clarify that the image registry can only be used for user application images and cannot be used for the OpenShift release or OLM operators images, see https://issues.redhat.com/browse/RFE-2488

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have addressed this comment in the current preview.

@tmulquee tmulquee force-pushed the TELCODOCS-597 branch 5 times, most recently from 966e074 to 345c6b7 Compare November 8, 2022 10:42
@tmulquee
Copy link
Contributor Author

tmulquee commented Nov 9, 2022

/label telco

@openshift-ci openshift-ci bot added the telco Label for all Telco PRs label Nov 9, 2022

This is unavoidable for the initial deployment. Over time, there is a risk that `crio` will wipe `/var/lib/containers/storage` in the case of a disorderly shutdown.

To address this, you need a local image registry on the SNO. The cluster images need to be mirrored to this local image registry along with being used as the registry for the applications. This is useful in Edge scenarios where an SNO is deployed on the Far Edge. The SNO cannot access the internet, just its own resources and eventually the internal network.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove The cluster images need to be mirrored to this local image registry since it's in contradiction with the NOTE section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

[id="ztp-configuring-pgt-image-registry_{context}"]
= Configuring the image registry using PolicyGenTemplate CRs

You can configure PTP fast events for vRAN clusters that are deployed using the GitOps Zero Touch Provisioning (ZTP) pipeline. Use `PolicyGenTemplate` custom resources (CRs) as the basis to create a hierarchy of configuration files tailored to your specific site requirements.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think PTP fast events are related to this section.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

annotations:
ran.openshift.io/ztp-deploy-wave: "100" <1>
# persistent volume claim
- fileName: StoragePVC.yaml
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line doesn't have the same indentation level with line 28

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tmulquee it needs an additional space. It does not render correctly in your preview.

@tmulquee
Copy link
Contributor Author

Marius edits implemented.

[id="ztp-configuring-disk-partitioning_{context}"]
= Configuring disk partitioning with SiteConfig

You can use the `SiteConfig` YAML file to generate the `MachineConfig` CR used for disk partitioning. You need to to modify values in the `MachineConfig` CR to reflect dependencies on the underlying disk.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to to modify values in the MachineConfig CR to reflect dependencies on the underlying disk. - imo this statement is not clear. The user entrypoint is the SiteConfig and that's where the disk related values are added. The user doesn't edit MachineConfig directly as they get generated from the SiteConfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


[NOTE]
====
The local image registry can only be used for user application images and cannot be used for the {product-title} or Operator Lifecycle Manager operator images. For details on working with these, see the information on the Topology Aware Lifecycle Manager (TAWLM) in Additional resources.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Topology Aware Lifecycle Manager (TAWLM) - we usually refer to it as TALM, not TAWLM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

The local image registry can only be used for user application images and cannot be used for the {product-title} or Operator Lifecycle Manager operator images. For details on working with these, see the information on the Topology Aware Lifecycle Manager (TAWLM) in Additional resources.
====

You can use the `SiteConfig` YAML file to generate the `MachineConfig` CR used for disk partitioning. Then use `PolicyGenTemplate` to apply to create the PV and PVC and patch `imageregistry` configuration.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we be a little more explicit here? Start with "This involves 2 steps: step1: Siteconfig... step 2: PGT"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rephrased as:
The first phase of setting up a local image registry is configuring disk partitioning with SiteConfig. You can use the SiteConfig YAML file to generate the MachineConfig CR used for disk partitioning.
Then next phase is to configure the image registry using PolicyGenTemplate CRs. You can use PolicyGenTemplate to apply to create the PV and PVC and patch imageregistry configuration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

size: 102500 <2>
start: 344844 <3>
----
<1> Depends on the hardware. The setting can also be a serial number or device name, for example `wwn`. The value must match with the `win` entry for `rootDeviceHint`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 - the example is already using wwn (remove "for example wwn")
2- "The value must match with the win entry for rootDeviceHint" change to "The value must match with the entry for rootDeviceHint" (drop 'win')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

----
<1> Remove this when moved to the correct PGT (`site/group/common`).
<2> Remove this when moved to the correct PGT (`site/group/common`).
<3> This assumes that `mount_point` is set to `/var/imageregistry` in `SiteConfig` using StorageClass `image-registry-sc` (see the first sc-file).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change "see the first sc-file" to something like See "StieConfig from Step 1"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to (see the topic on configuring disk partitioning with SiteConfig)


You can use `PolicyGenTemplate` to apply to create the PV and PVC and patch `imageregistry` configuration. Select the appropriate `PolicyGenTemplate` for each `source-cr`. See Additional Resources for more help.

.Prerequisites

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally....the pre req is that the user has ZTP setup

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added "* You have installed and configured Zero Touch Provisioning (ZTP)."

include::modules/ztp-configuring-pgt-image-registry.adoc[leveloffset=+3]

[role="_additional-resources"]
.Additional resources

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Include ZTP setup process doc here (along with the existing TALM and Image Registry doc)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added "For further information on this, see the topic on ZTP in Additional resources. "

and

@tmulquee tmulquee force-pushed the TELCODOCS-597 branch 3 times, most recently from dc1a6a6 to 4f87fe6 Compare November 16, 2022 14:46
Copy link
Contributor

@aireilly aireilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added xrefs based on your suggestions :)

@tmulquee tmulquee force-pushed the TELCODOCS-597 branch 2 times, most recently from 8359522 to 74244d8 Compare November 30, 2022 14:19
Copy link
Contributor

@aireilly aireilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 2 comments on the size and start field descriptions :)

modules/ztp-configuring-disk-partitioning.adoc Outdated Show resolved Hide resolved
modules/ztp-configuring-disk-partitioning.adoc Outdated Show resolved Hide resolved
@tmulquee
Copy link
Contributor Author

tmulquee commented Dec 1, 2022

All comments implemented.

@tmulquee
Copy link
Contributor Author

tmulquee commented Dec 1, 2022

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label Dec 1, 2022
@mburke5678 mburke5678 added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-in-progress Signifies that the merge review team is reviewing this PR labels Dec 1, 2022
@tmulquee
Copy link
Contributor Author

tmulquee commented Dec 1, 2022

Sorry, needed one more push to implement final edits.

Copy link
Contributor

@aireilly aireilly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Tony, noticed an xref is causing the build to fail, also spotted a few more things.

modules/ztp-configuring-pgt-image-registry.adoc Outdated Show resolved Hide resolved
modules/ztp-configuring-pgt-image-registry.adoc Outdated Show resolved Hide resolved
modules/ztp-configuring-pgt-image-registry.adoc Outdated Show resolved Hide resolved
modules/ztp-configuring-pgt-image-registry.adoc Outdated Show resolved Hide resolved
modules/ztp-configuring-pgt-image-registry.adoc Outdated Show resolved Hide resolved
@mburke5678 mburke5678 removed the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Dec 1, 2022
@tmulquee
Copy link
Contributor Author

tmulquee commented Dec 2, 2022

Edits complete. Applying once more for merge.

@tmulquee
Copy link
Contributor Author

tmulquee commented Dec 2, 2022

/label merge-review-needed

@mjpytlak
Copy link
Contributor

mjpytlak commented Dec 5, 2022

@tmulquee I noticed the do not merge-hold label. Just looking to confirm you want this merged. Thanks.

@tmulquee
Copy link
Contributor Author

tmulquee commented Dec 5, 2022

Yes please Mike. I didn't know whether I had the rights to move that hold label. All edits are done.

@mjpytlak mjpytlak added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. merge-review-needed Signifies that the merge review team needs to review this PR labels Dec 5, 2022
@mjpytlak mjpytlak added this to the Planned for 4.12 GA milestone Dec 5, 2022
@mjpytlak mjpytlak merged commit d38ec9f into openshift:main Dec 5, 2022
@mjpytlak
Copy link
Contributor

mjpytlak commented Dec 5, 2022

/cherrypick enterprise-4.12

@openshift-cherrypick-robot

@mjpytlak: new pull request created: #53480

In response to this:

/cherrypick enterprise-4.12

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.12 merge-review-in-progress Signifies that the merge review team is reviewing this PR peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files. telco Label for all Telco PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants