Apply required ospdo changes to adoption docs #855

pinikomarov · 2025-03-15T23:56:50Z

Jira: https://issues.redhat.com/browse/OSPRH-14644

softwarefactory-project-zuul · 2025-03-15T23:56:57Z

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/openstack-k8s-operators/data-plane-adoption for 855,18be40ff97bbea9d21671c391190404abe2fe9d1

docs_user/modules/proc_adopting-compute-services-to-the-data-plane.adoc

bogdando

see inline

docs_user/modules/proc_migrating-databases-to-mariadb-instances.adoc

jistr · 2025-03-17T13:00:15Z

docs_user/modules/proc_preparing-controller-nodes-for-director-operator-adoption.adoc

@@ -20,23 +20,23 @@ $ oc project openstack
 . Define the common environment variables:
 +
 ----
-export PASSWORD_FILE="tripleo-passwords.yaml"
+$ export PASSWORD_FILE="tripleo-passwords.yaml"


As discussed on the call please separate formatting refactorings from factual changes into different PRs.

I'll go over and remove any "cosmetic" chanegs

docs_user/modules/proc_migrating-ovn-data.adoc

klgill

A general comment on the docs: The ospdo adoption preview is not rendering properly. I noticed the following variables in many of the modules again:

:leveloffset: 2
:leveloffset: +1

I think these ^ are causing the formatting issues. Please remove these variables.

klgill · 2025-03-17T15:17:18Z

docs_user/modules/con_adoption-limitations.adoc

@@ -29,7 +29,7 @@ The following {compute_service_first_ref} features are Technology Previews:
 * AMD SEV
 * Direct download from Rados Block Device (RBD)
 * File-backed memory
-* `Provider.yaml`
+** Multiple data plane node sets


Is the "Multiple data plane node sets" bullet supposed to be placed under the Compute features?
I also don't think it's supposed to be a sub-bullet of "File-backed memory", so I removed one asterisk.

Suggested change

** Multiple data plane node sets

* Multiple data plane node sets

Does this limitation only apply to both ospdo and regular adoption?

not sure TBH, that filename seems like a typo, since this PR is for crucial changes only , I'm removing this change

klgill · 2025-03-17T15:21:12Z

docs_user/modules/con_adoption-process-overview.adoc

@@ -9,6 +9,9 @@ Familiarize yourself with the steps of the adoption process and the optional pos
 . xref:migrating-tls-everywhere_configuring-network[Migrate TLS everywhere (TLS-e) to the Red Hat OpenStack Services on OpenShift (RHOSO) deployment].
 . xref:migrating-databases-to-the-control-plane_configuring-network[Migrate your existing databases to the new control plane].
 . xref:adopting-openstack-control-plane-services_configuring-network[Adopt your Red Hat OpenStack Platform 17.1 control plane services to the new RHOSO 18.0 deployment].
+ifeval::["{build_variant}" == "ospdo"]
+. xref:ospdo_scale_down_pre_database_adoption_configuring-network[Scaling down director Operator resources].


Suggested change

. xref:ospdo_scale_down_pre_database_adoption_configuring-network[Scaling down director Operator resources].

. xref:ospdo-scale-down-pre-database-adoption_configuring-network[Scaling down director Operator resources].

docs_user/modules/proc_ospdo-scale-down-pre-database-adoption.adoc

klgill · 2025-03-17T15:24:37Z

docs_user/modules/con_comparing-configuration-files-between-deployments.adoc

@@ -3,6 +3,10 @@
 = Comparing configuration files between deployments

 To help you manage the configuration for your {OpenStackPreviousInstaller} and {rhos_prev_long} ({OpenStackShort}) services, you can compare the configuration files between your {OpenStackPreviousInstaller} deployment and the {rhos_long} cloud by using the os-diff tool.
+ifeval::["{build_variant}" == "ospdo"]
+[NOTE]
+ At this time Os-diff does not support director Operator.


Suggested change

At this time Os-diff does not support director Operator.

Os-diff does not currently support director Operator.

klgill · 2025-03-17T15:28:17Z

docs_user/modules/proc_adopting-image-service-with-ceph-backend.adoc

@@ -55,6 +55,11 @@ EOF
 If you backed up your {rhos_prev_long} ({OpenStackShort}) services configuration file from the original environment, you can compare it with the confgiuration file that you adopted and ensure that the configuration is correct.
 For more information, see xref:pulling-configuration-from-tripleo-deployment_adopt-control-plane[Pulling the configuration from a {OpenStackPreviousInstaller} deployment].

+ifeval::["{build_variant}" == "ospdo"]
+[NOTE]
+ At this time Os-diff does not support director Operator.


Suggested change

At this time Os-diff does not support director Operator.

Os-diff does not currently support director Operator.

klgill · 2025-03-17T16:28:51Z

docs_user/modules/proc_preparing-controller-nodes-for-director-operator-adoption.adoc

+$ oc adm uncordon $OSP18_NODE1
+$ oc adm uncordon $OSP18_NODE2
+----
+


Suggested change

+

klgill · 2025-03-17T16:37:53Z

docs_user/modules/proc_stopping-openstack-services.adoc

+    ifeval::["{build_variant}" != "ospdo"]
    for i in {1..3}; do
        SSH_CMD=CONTROLLER${i}_SSH
+    endif::[]
+    ifeval::["{build_variant}" == "ospdo"]
+        SSH_CMD=CONTROLLER_SSH
+    endif::[]


Suggested change

ifeval::["{build_variant}" != "ospdo"]

for i in {1..3}; do

SSH_CMD=CONTROLLER${i}_SSH

endif::[]

ifeval::["{build_variant}" == "ospdo"]

SSH_CMD=CONTROLLER_SSH

endif::[]

ifeval::["{build_variant}" != "ospdo"]

for i in {1..3}; do

SSH_CMD=CONTROLLER${i}_SSH

endif::[]

ifeval::["{build_variant}" == "ospdo"]

SSH_CMD=CONTROLLER_SSH

endif::[]

The ifeval statements are rendering in the preview. Removing the extra spaces should fix it.

klgill · 2025-03-17T16:38:12Z

docs_user/modules/proc_stopping-openstack-services.adoc

+    ifeval::["{build_variant}" != "ospdo"]
    done
+    endif::[]


Suggested change

ifeval::["{build_variant}" != "ospdo"]

done

endif::[]

ifeval::["{build_variant}" != "ospdo"]

done

endif::[]

klgill · 2025-03-17T16:40:02Z

docs_user/modules/proc_stopping-openstack-services.adoc

+    ifeval::["{build_variant}" != "ospdo"]
+    done
+    endif::[]


Suggested change

ifeval::["{build_variant}" != "ospdo"]

done

endif::[]

ifeval::["{build_variant}" != "ospdo"]

done

endif::[]

klgill · 2025-03-17T16:40:32Z

docs_user/modules/proc_stopping-openstack-services.adoc

+    ifeval::["{build_variant}" != "ospdo"]
+    done
+    endif::[]


Suggested change

ifeval::["{build_variant}" != "ospdo"]

done

endif::[]

ifeval::["{build_variant}" != "ospdo"]

done

endif::[]

pinikomarov · 2025-03-17T23:54:09Z

I believe the compiled resources look ok :
https://logserver.rdoproject.org/55/855/ac1dd42bb9cce45f2f49da39a9d03e1e2d61e942/github-check/adoption-docs-preview/98de71f/docs_build/docs_build/adoption-user/index-downstream-ospdo.html

jistr

Other than the indentation issue and Katie's comments, the patch LGTM.

jistr · 2025-03-18T07:24:01Z

docs_user/modules/proc_stopping-openstack-services.adoc

-                ${!SSH_CMD} sudo systemctl stop $service
-            fi
+ifeval::["{build_variant}" != "ospdo"]
+for i in {1..3}; do


This still breaks indentation of the code example. See the indentation on what you're removing vs on what you're adding.

softwarefactory-project-zuul · 2025-03-18T09:14:13Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/c17ec7b4e7d64549865dd564079e3cfe

✔️ noop SUCCESS in 0s
❌ adoption-docs-preview FAILURE in 1m 28s

jistr · 2025-03-18T09:38:37Z

docs_user/modules/proc_stopping-openstack-services.adoc

+    if [ ! -z "${!SSH_CMD}" ]; then
+        echo "Stopping the $service in controller $i"
+        if ${!SSH_CMD} sudo systemctl is-active $service; then
+            ${!SSH_CMD} sudo systemctl stop $service


This still un-indents things for the default guide. Let's please keep the original indentation, it will also make the diff cleaner like in the other places where we do similar edits.

ok updated that to indent correctly

I don't think it's fixed... If it was, the lines of diff would disappear. The outermost if line is indented by 4 spaces but it was previously indented by 8 spaces and i think we should keep that, to make it appear correct in the for cycle.

See how you did it in proc_migrating-ovn-data.adoc, i think there it is correct.

If i look at the docs preview for this PR, this is how it renders:

echo "Stopping systemd OpenStack services" for service in ${ServicesToStop[*]}; do for i in {1..3}; do SSH_CMD=CONTROLLER${i}_SSH if [ ! -z "${!SSH_CMD}" ]; then echo "Stopping the $service in controller $i" if ${!SSH_CMD} sudo systemctl is-active $service; then ${!SSH_CMD} sudo systemctl stop $service fi fi done done

updated indentation , I believe its better now , now it's looking like this :

bogdando · 2025-03-18T10:30:39Z

docs_user/modules/proc_migrating-databases-to-mariadb-instances.adoc

@@ -26,6 +26,11 @@ $ STORAGE_CLASS=local-storage
 $ MARIADB_IMAGE=registry.redhat.io/rhoso/openstack-mariadb-rhel9:18.0
 endif::[]

+ifeval::["{build_variant}" == "ospdo"]
+$ MARIADB_CLIENT_ANNOTATIONS='[{"name": internalapi-static, "ips": ["10.2.120.9/24"]}]'


please align this with tests https://github.com/bogdando/data-plane-adoption/blob/aba800cfcdbd83f6a602f19d465743a47465bacc/tests/roles/common_defaults/defaults/main.yaml#L127

bogdando

Please align MARIADB_CLIENT_ANNOTATIONS in tests and docs

klgill

@pinikomarov I just have 2 very minor formatting changes for you to implement. Otherwise, this LGTM.

klgill · 2025-03-18T14:01:02Z

docs_user/modules/proc_migrating-ovn-data.adoc

+$ $CONTROLLER_SSH ip a s enp4s0
+** Select the non /32 IP address
+----
+


Suggested change

+

klgill · 2025-03-18T14:02:00Z

docs_user/modules/proc_ospdo-scale-down-pre-database-adoption.adoc

+    $OS_CLIENT cat /home/cloud-admin/.ssh/id_rsa.pub > temp/ssh.pub
+    echo "SSH private and public keys saved in temp/ssh.private and temp/ssh.pub"
+----
+. Get the OVN configuration from each Compute node role, `OpenStackBaremetalSet`. This configuration is  used later to build the `OpenStackDataPlaneNodeSet`(s). Repeat the following step for each `OpenStackBaremetalSet `:


Suggested change

. Get the OVN configuration from each Compute node role, `OpenStackBaremetalSet`. This configuration is used later to build the `OpenStackDataPlaneNodeSet`(s). Repeat the following step for each `OpenStackBaremetalSet `:

. Get the OVN configuration from each Compute node role, `OpenStackBaremetalSet`. This configuration is used later to build the `OpenStackDataPlaneNodeSet`(s). Repeat the following step for each `OpenStackBaremetalSet`:

Jira: https://issues.redhat.com/browse/OSPRH-14644

jistr

/lgtm

pinikomarov · 2025-03-19T20:41:42Z

@jistr seems it needs approved label ?

openshift-ci · 2025-03-19T20:52:37Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by:

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

The pull request process is described here

Needs approval from an approver in each of these files:

OWNERS

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

klgill · 2025-03-19T20:53:20Z

@pinikomarov I just added the approved label, since all reviewers have given their blessings.

openshift-merge-robot added the needs-rebase label Mar 15, 2025

pinikomarov force-pushed the adoption_ospdo_docs_changes branch from 18be40f to bb37598 Compare March 16, 2025 00:24

openshift-merge-robot removed the needs-rebase label Mar 16, 2025

bogdando reviewed Mar 17, 2025

View reviewed changes

docs_user/modules/proc_adopting-compute-services-to-the-data-plane.adoc Outdated Show resolved Hide resolved

bogdando requested changes Mar 17, 2025

View reviewed changes

bogdando reviewed Mar 17, 2025

View reviewed changes

docs_user/modules/proc_migrating-databases-to-mariadb-instances.adoc Outdated Show resolved Hide resolved

bogdando reviewed Mar 17, 2025

View reviewed changes

docs_user/modules/proc_migrating-databases-to-mariadb-instances.adoc Outdated Show resolved Hide resolved

bogdando reviewed Mar 17, 2025

View reviewed changes

docs_user/modules/proc_migrating-databases-to-mariadb-instances.adoc Show resolved Hide resolved

jistr reviewed Mar 17, 2025

View reviewed changes

bogdando reviewed Mar 17, 2025

View reviewed changes

docs_user/modules/proc_migrating-ovn-data.adoc Outdated Show resolved Hide resolved

klgill requested changes Mar 17, 2025

View reviewed changes

pinikomarov force-pushed the adoption_ospdo_docs_changes branch 2 times, most recently from 67c4201 to ac1dd42 Compare March 17, 2025 23:29

jistr reviewed Mar 18, 2025

View reviewed changes

pinikomarov force-pushed the adoption_ospdo_docs_changes branch from ac1dd42 to 3176475 Compare March 18, 2025 09:12

pinikomarov force-pushed the adoption_ospdo_docs_changes branch from 3176475 to 0c79371 Compare March 18, 2025 09:26

jistr reviewed Mar 18, 2025

View reviewed changes

pinikomarov force-pushed the adoption_ospdo_docs_changes branch from 0c79371 to d38e7c2 Compare March 18, 2025 10:08

bogdando reviewed Mar 18, 2025

View reviewed changes

bogdando requested changes Mar 18, 2025

View reviewed changes

klgill requested changes Mar 18, 2025

View reviewed changes

pinikomarov force-pushed the adoption_ospdo_docs_changes branch 3 times, most recently from a07863f to 853f6fc Compare March 19, 2025 13:07

Apply required ospdo changes to adoption docs

09f90d5

Jira: https://issues.redhat.com/browse/OSPRH-14644

pinikomarov force-pushed the adoption_ospdo_docs_changes branch from 853f6fc to 09f90d5 Compare March 19, 2025 13:12

jistr reviewed Mar 19, 2025

View reviewed changes

openshift-ci bot assigned jistr Mar 19, 2025

openshift-ci bot added the lgtm label Mar 19, 2025

klgill added the approved label Mar 19, 2025

openshift-merge-bot bot merged commit 3c6bf64 into openstack-k8s-operators:main Mar 19, 2025
6 checks passed

pinikomarov deleted the adoption_ospdo_docs_changes branch March 20, 2025 07:35

	** Multiple data plane node sets
	* Multiple data plane node sets

	. xref:ospdo_scale_down_pre_database_adoption_configuring-network[Scaling down director Operator resources].
	. xref:ospdo-scale-down-pre-database-adoption_configuring-network[Scaling down director Operator resources].

	At this time Os-diff does not support director Operator.
	Os-diff does not currently support director Operator.

	. Get the OVN configuration from each Compute node role, `OpenStackBaremetalSet`. This configuration is used later to build the `OpenStackDataPlaneNodeSet`(s). Repeat the following step for each `OpenStackBaremetalSet `:
	. Get the OVN configuration from each Compute node role, `OpenStackBaremetalSet`. This configuration is used later to build the `OpenStackDataPlaneNodeSet`(s). Repeat the following step for each `OpenStackBaremetalSet`:

Apply required ospdo changes to adoption docs #855

Apply required ospdo changes to adoption docs #855

Conversation

pinikomarov commented Mar 15, 2025

softwarefactory-project-zuul bot commented Mar 15, 2025

bogdando left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klgill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pinikomarov commented Mar 17, 2025

jistr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

softwarefactory-project-zuul bot commented Mar 18, 2025

Choose a reason for hiding this comment

pinikomarov Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

jistr Mar 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bogdando left a comment

Choose a reason for hiding this comment

klgill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jistr left a comment

Choose a reason for hiding this comment

pinikomarov commented Mar 19, 2025

openshift-ci bot commented Mar 19, 2025

klgill commented Mar 19, 2025

pinikomarov Mar 18, 2025 •

edited

Loading

jistr Mar 18, 2025 •

edited

Loading