[disconnected] Create hook to configure disconnected cluster#3814
[disconnected] Create hook to configure disconnected cluster#3814drosenfe wants to merge 1 commit into
Conversation
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3bfbd3399c6a4ba885dea99111ee34ba ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 13m 20s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
6de4cf9 to
7310e9e
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/a9418611826d43ccbda9dbdbbe8f99a3 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 10m 37s |
slagle
left a comment
slagle
left a comment
There was a problem hiding this comment.
you'll also need another task to login the dataplane nodes to the mirror registry. you can add this to edpm_container_registry_logins somewhere.
drosenfe
force-pushed
the
disconnectedhook
branch
from
7310e9e to
81450ab
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/68ec32cda5c9495bb05bbd86f98bb58d ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 17m 30s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
81450ab to
086c8b8
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/3baa9443fe84436d94eb8f62bc6e270b ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 21m 04s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
086c8b8 to
0a67b84
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/b7f6831c3a38441e8b31d359cb133524 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 26m 40s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
0a67b84 to
22d1864
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/749f1d93ae1948dc838dbe448b531af2 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 15m 20s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
22d1864 to
8220076
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/79903bc23f894b8b8233fa0880a098ee ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 16m 06s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
8220076 to
1a9a351
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/d3c2d308a96844728a4c40a91fe44dbe ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 19m 03s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
1a9a351 to
b4a5652
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/fa733e9d790e41f1b516d2829414f210 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 06m 55s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
b4a5652 to
72b53cb
Compare
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/6c682e2aa4c346d1a5abb87786fcb26a ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 20m 44s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
72b53cb to
163f026
Compare
|
recheck |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 13m 32s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
163f026 to
cb684fd
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 40m 33s |
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 21m 04s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
6b6eba3 to
c782420
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 21m 26s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
c782420 to
3b43ac9
Compare
|
recheck |
michburk
left a comment
michburk
left a comment
There was a problem hiding this comment.
There are a couple of places where you do straightforward operations with the oc command, is it possible to use modules like kubernetes.core.k8s/kubernetes.core.k8s_info instead? I know that not all instances of ansible.builtin.shell + oc ... can be translated as such, but if I'm not mistaken, some of the simpler oc apply/patch/create commands could.
Additionally, do you have a jira you could link here? And if you have run this in a testproject, we would greatly appreciate if that were attached to the jira ticket / in the comments of that jira ticket.
Thanks!
drosenfe
force-pushed
the
disconnectedhook
branch
from
3b43ac9 to
57ccbfb
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 15m 33s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
57ccbfb to
e9a4a88
Compare
@michburk looked at using module: kubernetes.core.k8s. The only line that was potentially supported was: oc apply -f {{ mirror_location }}/working-dir/cluster-resources. However, that is applying a directory rather than a file which the module kubernetes.core.k8s did not support. I believe all other comments were addressed. Are you OK with review now? |
michburk
left a comment
michburk
left a comment
There was a problem hiding this comment.
There are a couple of places where you do straightforward operations with the
occommand, is it possible to use modules likekubernetes.core.k8s/kubernetes.core.k8s_infoinstead? I know that not all instances ofansible.builtin.shell+oc ...can be translated as such, but if I'm not mistaken, some of the simpleroc apply/patch/createcommands could.
Additionally, do you have a jira you could link here? And if you have run this in a testproject, we would greatly appreciate if that were attached to the jira ticket / in the comments of that jira ticket.
Thanks!@michburk looked at using module: kubernetes.core.k8s. The only line that was potentially supported was: oc apply -f {{ mirror_location }}/working-dir/cluster-resources. However, that is applying a directory rather than a file which the module kubernetes.core.k8s did not support. I believe all other comments were addressed. Are you OK with review now?
A couple more things, very sorry for not catching them on the first pass. It's generally considered best practice to avoid shell where possible and prefer more specific modules or ansible.builtin.command if we have to use shell-like commands and don't need pipes/&&/||/etc.
I do want to follow-up and ask, is it the case that the tasks
Create update service namespace, Create update service operator group, and Create subscription service for example can't be converted to use kubernetes.core.k8s? Very sorry if I'm missing something obvious here.
I'll keep an eye on this pr to try to be more responsive, sorry for not seeing your comment yesterday.
| vars: | ||
| oc_mirror_download_url: "{{ cifmw_disconnected_mirror_url | default('https://mirror.openshift.com/pub/openshift-v4/x86_64/clients/ocp/latest/oc-mirror.rhel9.tar.gz') }}" | ||
| mirror_registry_url: "{{ cifmw_disconnected_registry_url | default('https://mirror.openshift.com/pub/cgw/mirror-registry/latest/mirror-registry-amd64.tar.gz') }}" | ||
| openstack_namespace: "{{ cifmw_openstack_namespace | default('openstack') }}" |
There was a problem hiding this comment.
openstack_namespace removed in latestet version.
|
|
||
| - name: Increase gunicorn-web timeout in quay-app container | ||
| ansible.builtin.command: >- | ||
| podman exec -it quay-app {% raw %}sed -i '/command=gunicorn |
There was a problem hiding this comment.
Just checking for my own understanding: are the -it flags on each of these podman exec commands necessary?
There was a problem hiding this comment.
I removed the -it options in latest version and it still worked.
| - name: Configure system to trust mirror registry root ca | ||
| become: true | ||
| ansible.builtin.shell: | | ||
| cp {{ local_registry }}/quay-rootCA/rootCA.pem /etc/pki/ca-trust/source/anchors/ |
There was a problem hiding this comment.
Could be split into two tasks, ansible.builtin.copy this file and a ansible.builtin.command task for update-ca-trust extract.
There was a problem hiding this comment.
Fixed in latest version.
|
|
||
| - name: Configure cluster to trust mirror registry root ca | ||
| ansible.builtin.shell: | | ||
| set -eux |
There was a problem hiding this comment.
could be split into two separate ansible.builtin.command tasks
There was a problem hiding this comment.
Fixed in latest version.
drosenfe
force-pushed
the
disconnectedhook
branch
from
e9a4a88 to
54d8166
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 17m 41s |
drosenfe
force-pushed
the
disconnectedhook
branch
from
54d8166 to
3ef0282
Compare
|
Build failed (check pipeline). Post ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 19m 06s |
Create a hook to configure openshift cluster for disconnected deployment. The hook should execute after the cluster is deployed but before openstack is deployed. jira: https://redhat.atlassian.net/browse/OSPRH-21316 Signed-off-by: David Rosenfeld drosenfe@redhat.com
drosenfe
force-pushed
the
disconnectedhook
branch
from
3ef0282 to
2bf98a4
Compare
|
michburk
left a comment
michburk
left a comment
There was a problem hiding this comment.
There are a number of places where you use ansible.builtin.command as with the command string on a new line without a cmd parameter (picking a random example):
- name: Wait until the OpenShift cluster is stable
ansible.builtin.command:
oc adm wait-for-stable-cluster --minimum-stable-period=5s --timeout=30m
where the following is preferred:
- name: Wait until the OpenShift cluster is stable
ansible.builtin.command:
cmd: oc adm wait-for-stable-cluster --minimum-stable-period=5s --timeout=30m
Sorry for not catching this earlier, I suppose the first method has worked in your testing, but style-wise the command should either be on the same line as ansible.builtin.command i.e.
ansible.builtin.command: echo hi
or preceded by cmd: , i.e.
ansible.builtin.command:
cmd: echo hi
really sorry if I'm dragging this along and not catching everything in earlier reviews, patch is generally looking good to me and I keep catching style nits. Sorry about that.
| - name: Install oc mirror | ||
| ansible.builtin.shell: | | ||
| set -eux | ||
| chmod +x {{ disconnect_working_dir }}/oc-mirror && | ||
| sudo mv {{ disconnect_working_dir }}/oc-mirror /usr/local/bin/. |
There was a problem hiding this comment.
Should probably be two tasks, where the mv ... command should use become: true instead of sudo
| - name: Apply yaml files from results directory to cluster | ||
| ansible.builtin.shell: | | ||
| oc apply -f {{ mirror_location }}/working-dir/cluster-resources | ||
|
|
||
| - name: Wait for mirrored operators to be available | ||
| ansible.builtin.shell: | | ||
| oc get packagemanifests.packages.operators.coreos.com | ||
| register: packagemanifest_out | ||
| until: | ||
| - "'openstack-operator' in packagemanifest_out.stdout" | ||
| - "'kubernetes-nmstate-operator' in packagemanifest_out.stdout" | ||
| retries: 10 | ||
| delay: 30 |
There was a problem hiding this comment.
these could be ansible.builtin.command instead of ansible.builtin.shell
| oc mirror --v2 --config {{ disconnect_working_dir }}/imageset-config-v2.yaml file://{{ mirror_location }} >>{{ disconnect_working_dir }}/mirror_images.log | ||
| register: mirror_image_result | ||
| until: mirror_image_result is not failed | ||
| retries: 1 |
There was a problem hiding this comment.
not blocking or anything and sorry if I'm missing something, but is 1 retry with no delay intentional here? Just want to double check
| oc mirror --v2 --config {{ disconnect_working_dir }}/imageset-config-v2.yaml --from file://{{ mirror_location }} docker://{{ host_fqdn.stdout }}:8443 >>{{ disconnect_working_dir }}/mirror_contents.log | ||
| register: mirror_contents_result | ||
| until: mirror_contents_result is not failed | ||
| retries: 1 |
There was a problem hiding this comment.
same comment about 1 retry with no delay
|
|
||
| - name: Disable catalog source | ||
| ansible.builtin.shell: | | ||
| oc patch OperatorHub cluster --type json -p '[{"op": "add", "path": "/spec/disableAllDefaultSources", "value": true}]' |
There was a problem hiding this comment.
this could be ansible.builtin.command instead of ansible.builtin.shell
4 participants
Create a hook to configure openshift cluster for disconnected deployment. The hook should execute after the cluster is deployed but before openstack is deployed.
jira: https://redhat.atlassian.net/browse/OSPRH-21316
Signed-off-by: David Rosenfeld drosenfe@redhat.com