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

Initial commit for mta v6.2.2 and v7.0.2 manifests #266

Merged
merged 7 commits into from
Jul 21, 2024

Conversation

rhkp
Copy link
Collaborator

@rhkp rhkp commented May 6, 2024

Hi @dmartinol / @masayag ,
Please review and merge or provide feedback.

This is part 2 (instructions 5-7) as provided at https://github.com/parodos-dev/serverless-workflows?tab=readme-ov-file#to-introduce-a-new-workflow

Thanks.

@dmartinol
Copy link
Collaborator

it's a pity that we have a lot of duplicated code, but probably acceptable in this context. Maube Moti and Gabriel have a different opinion on this topic.
maybe the mta folder is useless and can be removed?
I would also remove all the initial manifests generated by the serverless-workflows automation, because they miss the additional manipulation performed by the GH action (e.g., setting prod profile, adding persistence and image configuration)

@masayag
Copy link
Collaborator

masayag commented May 29, 2024

@rhkp is this PR rebased on the workflows branch? Can we adjust the versions to match the one from the origin repository?

@rhkp
Copy link
Collaborator Author

rhkp commented May 29, 2024

@rhkp is this PR rebased on the workflows branch? Can we adjust the versions to match the one from the origin repository?

Sure. Let me do that. Thanks.

@rhkp
Copy link
Collaborator Author

rhkp commented May 29, 2024

@masayag Thanks for the comment.
Submitting the updated manifests based on changes in underlying MTA workflows including changes to the dir names.
Please let me know of any changes that you may wish me to do or approve.

Copy link
Collaborator

@masayag masayag left a comment

Choose a reason for hiding this comment

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

Sorry for missing this part, but I can't see the container image built for those workflows from the serverless-workflows repository specified in the sonataflow CRs.

duration: PT1H
podTemplate:
container:
resources: {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

the image should be specified here, in a similar way as done for the existing mta workflow:
https://github.com/parodos-dev/serverless-workflows-config/blob/main/charts/mta/templates/01-sonataflow_mtaanalysis.yaml#L252

so it appears the repository was never created and the image wasn't pushed: https://github.com/parodos-dev/serverless-workflows/actions/workflows/mta-v6.x.yml

I'm going to create the repositories in quay for both workflows (v6 and v7) and repeat the failed actions.
Only then this CR can be updated.

Copy link
Collaborator

Choose a reason for hiding this comment

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

now things looks better - there is an image created successfully at quay.io/orchestrator/serverless-workflow-mta-v6.x:1bde42520bd287ded1eab55380a2d17912f0f32a
and it needs to be added here.

then, by merging this PR, the next failed task in the job below should be resolved:
https://github.com/parodos-dev/serverless-workflows/actions/runs/9552838333/job/26330499375

Copy link
Collaborator

Choose a reason for hiding this comment

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

in the meantime, running the build for mta-v7

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.

Remove the 01-XX 02-XX and 03-XX files, they will be generated by the CI

mta.url = ${MTA_URL:http://tackle-ui.my-konveyor-operator.svc.cluster.local:8080}
quarkus.rest-client.mta_json.url = ${mta.url}
quarkus.rest-client.notifications.url=${BACKSTAGE_NOTIFICATIONS_URL:http://backstage-backstage.rhdh-operator/api/notifications/}
quarkus.openapi-generator.mta_json.auth.bearerAuth.bearer-token=${MTA_HUB_TOKEN}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this and how to generate one?
There is no such props in https://github.com/parodos-dev/serverless-workflows/blob/main/mta-v7.x/application.properties

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These were generated using make command with gen-manifests option.

https://github.com/parodos-dev/serverless-workflows/blob/main/Makefile#L173

rhkp and others added 7 commits July 21, 2024 10:13
Set unique name for the chart.
mta is the name of the existing MTA workflow chart.
Set unique name for the mta v6 chart.
mta is the name of the existing MTA workflow chart.
Signed-off-by: Moti Asayag <masayag@redhat.com>

rh-pre-commit.version: 2.3.0
rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Moti Asayag <masayag@redhat.com>

rh-pre-commit.version: 2.3.0
rh-pre-commit.check-secrets: ENABLED
Signed-off-by: Moti Asayag <masayag@redhat.com>

rh-pre-commit.version: 2.3.0
rh-pre-commit.check-secrets: ENABLED
@masayag masayag merged commit 5ee4f95 into parodos-dev:main Jul 21, 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.

None yet

4 participants