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

Add an initial StepAction for ephemeral cluster provisioning #1058

Merged
merged 4 commits into from
Jun 19, 2024

Conversation

amisstea
Copy link
Contributor

@amisstea amisstea commented Jun 6, 2024

No description provided.

@amisstea amisstea force-pushed the RHTAPWATCH-1022-1 branch from b00eef2 to 22d32dd Compare June 6, 2024 02:35
@amisstea
Copy link
Contributor Author

amisstea commented Jun 6, 2024

@Omeramsc please review. @lcarva I believe you've been looking at StepActions recently so it'd be great to get your eyes on this too.

@amisstea amisstea requested a review from Omeramsc June 6, 2024 13:29
Copy link

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

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

LGTM

@amisstea amisstea marked this pull request as draft June 7, 2024 15:22
@amisstea
Copy link
Contributor Author

amisstea commented Jun 7, 2024

Moving to draft state. I'm planning to make changes to the params based on a recent decision to make the EaaS subspaces ephemeral.

@amisstea amisstea marked this pull request as ready for review June 9, 2024 14:20
@amisstea amisstea requested a review from Omeramsc June 9, 2024 14:20
@amisstea
Copy link
Contributor Author

amisstea commented Jun 9, 2024

@Omeramsc please take another look at the latest changes. I'll open a separate PR for creation of the ephemeral EaaS space.

Copy link

@Omeramsc Omeramsc left a comment

Choose a reason for hiding this comment

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

LGTM

@amisstea
Copy link
Contributor Author

@konflux-ci/build-maintainers can I please get some feedback (and possibly an approval) on this PR?

@chmeliik
Copy link
Contributor

@konflux-ci/build-maintainers can I please get some feedback (and possibly an approval) on this PR?

I don't have any context, but I can check the Tekton-isms and give the green checkmark based on approval from your team

Though I would appreciate some basic context for why this is a StepAction and who/what will use it

@amisstea
Copy link
Contributor Author

Though I would appreciate some basic context for why this is a StepAction and who/what will use it

This StepAction is meant to be used with a few others (see #1067) for Konflux EaaS (Environment as a Service). They're a set of utilities which can be combined (as needed, or with custom steps) to provision ephemeral clusters from a pipelinerun. Use of StepActions, in particular, is a workaround (see here for the explanation). This StepAction could be a task but I think this way aligns with the motivations behind StepActions.

Copy link
Contributor

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@chmeliik
Copy link
Contributor

/retest

@chmeliik
Copy link
Contributor

The tests seem to be failing on

timed out when waiting for devfile content creation for application ...

That may be fixed by rebasing now that #1066 is in

amisstea added 4 commits June 18, 2024 12:04
SpaceRequests will be ephemeral so the secret name will be known when
incorporating this StepAction into a Task.

The StepAction name is prefixed with "eaas" for extra clarity about
its purpose.
@amisstea amisstea force-pushed the RHTAPWATCH-1022-1 branch from 2a6b525 to 3ddee8c Compare June 18, 2024 16:04
@amisstea
Copy link
Contributor Author

@chmeliik please merge this if you can.

@chmeliik chmeliik added this pull request to the merge queue Jun 19, 2024
Merged via the queue into konflux-ci:main with commit 45bc830 Jun 19, 2024
3 checks passed
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.

3 participants