MTV-1590 Improved hooks description #586
Conversation
RichardHoch
force-pushed
the
improved_hook_description
branch
2 times, most recently
from
c0e8df2 to
931c4a2
Compare
RichardHoch
changed the title
| + | ||
| [NOTE] | ||
| ==== | ||
| You can also use a `here` document to encode a playbook: |
There was a problem hiding this comment.
could you explain a bit more, as i got a bit confused?
There was a problem hiding this comment.
@anarnold97 I'm actually wondering if this is still relevant -- it was in the upstream document that I refashioned into the current downstream documentation. I'll make a note to ask Martin and Matthew about it when I ask for their review.
There was a problem hiding this comment.
I did change the wording.
There was a problem hiding this comment.
@mnecas Is this note about a here document still relevant?
documentation/modules/images/updated_hooks_figure_from_martin_cropped.png
Outdated
Show resolved
Hide resolved
RichardHoch
force-pushed
the
improved_hook_description
branch
2 times, most recently
from
aed7a60 to
76326e0
Compare
Signed-off-by: RichardHoch <[email protected]>
RichardHoch
force-pushed
the
improved_hook_description
branch
from
76326e0 to
01b65ab
Compare
|
Hi, @RichardHoch ! I made some comments to you directly via Slack. Happy to continue the conversation here, if needed. |
| [id="hook-execution_{context}"] | ||
| == Hook execution | ||
| An Ansible playbook that is provided as part of a migration hook is mounted into the hook container as a `ConfigMap`. The hook container is run as a job on the desired cluster, using the default `ServiceAccount` in the `konveyor-forklift` namespace. | ||
|
|
There was a problem hiding this comment.
@mnecas @matthewsecaur comments: The text says: "The hook container is run as a job on the desired cluster, using the default ServiceAccount in the konveyor-forklift namespace"
Issue: MTV is not installed in the konveyor-forklift namespace. It is installed in openshift-mtv. Also, the default serviceAccount is forklift-controller, though it might not be necessary to mention that.
| . For a pre-migration hook, perform the following steps: | ||
|
|
||
| .. In the *Pre migration hook* section, toggle the *Enable hook* switch to *Enable pre migration hook*. | ||
| .. Enter the *Hook runner image*. The default image is `quay.io/konveyor/hook-runner` and this value must be used if you are adding an Ansible playbook. |
There was a problem hiding this comment.
@mnecas @matthewsecaur comments:
The text says: "Enter the Hook runner image. The default image is quay.io/konveyor/hook-runner and this value must be used if you are adding an Ansible playbook."
Issue: I don't understand what we mean when we say "this value must be used if you are adding and Ansible playbook". Customers are free to use their own image, are they not? I suppose their custom image could be based on the default hook-runner image, by why do we say is MUST be used? Here is an example of a custom hook-runner image that can be used with a playbook: https://github.com/alezzandro/aap25-ee-vmware-rhel8/tree/main/hook-runner
| $ oc -n openshift-mtv patch hook <name_of_hook> \ | ||
| -p '{"spec":{"serviceAccount":"<service_account>"}}' --type merge | ||
| ---- | ||
|
|
There was a problem hiding this comment.
@mnecas @matthewsecaur notes:
Issue: The command is correct, but we don't go into any detail about how to create the serviceAccount. It is mentioned in the Prerequisites, so I suppose that is sufficient, but should there be some additional text? Perhaps instead of "associate each hook with your Red Hat OpenShift service account", we could say "associate each hook with the Red Hat OpenShift service account that was created for running hooks" or similar?
| EOF | ||
| ---- | ||
| ==== | ||
|
|
There was a problem hiding this comment.
@mnecas Do you have a better way to explain what a 'here" document is? Is this note actually needed/useful for our users?
| where: | ||
| + | ||
| `playbook` refers to an optional Base64-encoded Ansible Playbook. If you specify a playbook, the `image` must be `hook-runner`. | ||
| + |
There was a problem hiding this comment.
@mnecas @matthewsecaur comments:
If we're going to create a Hook CR here, then we may as well have the user put the serviceAccount in now, instead of creating the hook and then adding the SA later. In fact, I don't see any mention of the serviceAccount in the CLI-based instructions. This is required for Console-based and CLI-based hooks.
The example command for decoding a playbook from a hook uses the "konveyor-forklift" namespace, but MTV is installed in "openshift-mtv"
Same as earlier, the text says "If you specify a playbook, the image must be hook-runner", but that isn't necessarily true. You can use a custom image and a playbook.
|
@mnecas Matthew added some comments in Slack and I copy-pasted them here. |
MTV 2.8
Resolves https://issues.redhat.com/browse/MTV-1590 by revising section 6.4. of the MTV user guide, "https://docs.redhat.com/en/documentation/migration_toolkit_for_virtualization/2.7/html-single/installing_and_using_the_migration_toolkit_for_virtualization/index#adding-hooks-migration-plan-using-api"
Preview: https://file.corp.redhat.com/rhoch/improved_hook_description/html-single/#adding-hooks-mtv-migration-plan