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

kustomize deployment of all workflows #81

Merged
merged 10 commits into from
Feb 27, 2024
Merged

kustomize deployment of all workflows #81

merged 10 commits into from
Feb 27, 2024

Conversation

dmartinol
Copy link
Collaborator

@dmartinol dmartinol commented Feb 16, 2024

See description of changes in the newly added README.md (last section) for move2kube
PS: The manifests were copied from the latest generation in a local repo (main branch). There are little changes compared to the version in the Helm charts, I did not update it for now

Copy link
Collaborator

Choose a reason for hiding this comment

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

this will contains what? The content of INTALL.md?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for escalation I put a simple arch description with mermaid diagrams, see here. But we can remove it completely

Copy link
Collaborator

@gabriel-farache gabriel-farache left a comment

Choose a reason for hiding this comment

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

some minor changes but looks good!

Copy link
Collaborator

@gabriel-farache gabriel-farache left a comment

Choose a reason for hiding this comment

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

The CRD for the MTA operator (for Tackle resource) is missing and needs to be applied prior to applying the Tackle manifest
Also, as we will have a growing amount of workflows, it would be nice to have a Makfile like they did to built kogito-apps image (ie: https://github.com/apache/incubator-kie-kogito-images?tab=readme-ov-file#building-images) ==> may be another PR

kustomize/move2kube/base/config.properties Show resolved Hide resolved
kustomize/mta/operator/00-tackle-resources.yaml Outdated Show resolved Hide resolved
kustomize/move2kube/base/03-knative-resources.yaml Outdated Show resolved Hide resolved

You can monitor the deployment status with:
```bash
oc get sonataflow m2k -n sonataflow-infra -owide
Copy link
Collaborator

@gabriel-farache gabriel-farache Feb 19, 2024

Choose a reason for hiding this comment

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

should be

oc get sonataflow m2k -n ${TARGET_NS} -owide


And finally view the logs with:
```bash
oc logs -f -n sonataflow-infra -l app=m2k
Copy link
Collaborator

Choose a reason for hiding this comment

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

same as above, the namespace should be parameterized too (${TARGET_NS} instead of sonataflow-infra)

Copy link
Collaborator

@gabriel-farache gabriel-farache left a comment

Choose a reason for hiding this comment

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

Some more comments after trying to follow the README for M2K

kustomize/move2kube/README.md Show resolved Hide resolved
@dmartinol
Copy link
Collaborator Author

@gabriel-farache

Some more comments after trying to follow the README for M2K

Did you also apply the prerequisites?

  • Apply the required configuration described in the Configuration section of INSTALL.md

@dmartinol dmartinol changed the title Initial commit of move2kube project kustomize deployment of all workflows Feb 20, 2024
@gabriel-farache
Copy link
Collaborator

Did you also apply the prerequisites?

Apply the required configuration described in the Configuration section of INSTALL.md

No I missed that part
But as it is patch commands, maybe it could be added to the kustomize manifests?

@dmartinol
Copy link
Collaborator Author

No I missed that part But as it is patch commands, maybe it could be added to the kustomize manifests?
Yes, both oc patch and oc adm policy add-scc-to-user could be kustomized.
Tracking this with issue #86

Copy link
Collaborator

@gabriel-farache gabriel-farache left a comment

Choose a reason for hiding this comment

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

Almost there; IMO 1 missing steps for m2k deployment (update move2kube URLs and restart pods)

And I still have the error with the removal of the creationTimestamp

And finally view the logs with:
```bash
oc logs -f -n ${TARGET_NS} -l app=m2k
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO, even if you reference the INSTALL.md file, you should copy/paste the steps when you update the move2kube URLs: provide the command to do it or at least the location of the file and the props to edit and then put the kustomize apply command again to apply those and then the command to restart the m2k pods to reflect the changes of the CM to the pods

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not following you here: are you referring to the pre-installation steps or the fact that whenever you update the CM/Secret mounted to the Pod's envFrom the Pod is not restarted? (which is a different issue, and BTW tracked by apache/incubator-kie-kogito-serverless-operator#298)
Pls clarify

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the README to clarify that the MOVE2KUBE_URL and MOVE2KUBE_API properties must match the exposed Route, but I would not make it a 2 steps installation. On the contrary, I would be tempted to split the configuration in 2 to isolate the installation of Move2Kube that, BTW, may also be already installed in the cluster or anywhere else. But not today, of course 😉

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would be tempted to split the configuration in 2 to isolate the installation of Move2Kube

That would be fine for me too, like in move2kube.md too add/move some steps
But that is for later for sure

@gabriel-farache
Copy link
Collaborator

gabriel-farache commented Feb 27, 2024

We are good to go Houston! 🚀

@dmartinol dmartinol merged commit 6aa1fbe into parodos-dev:main Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants