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

Prow: Use CAPI Operator #917

Merged
merged 1 commit into from
Nov 27, 2024

Conversation

lentzi90
Copy link
Member

@lentzi90 lentzi90 commented Nov 26, 2024

We have so far been relying on imperative commands to manage the lifecycle of CAPI/CAPO and cert-manager. This commit introduces the CAPI Operator with manifests to declaratively manage these components instead.

Fixes: #907

@metal3-io-bot metal3-io-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 26, 2024
@lentzi90
Copy link
Member Author

/hold
I want to make sure everyone is fine with the way this works before we merge.
Changes have not been applied yet.

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 26, 2024
@tuminoid
Copy link
Member

/cc @adilGhaffarDev @kashifest
CAPI guys PTAL.

We have so far been relying on imperative commands to manage the
lifecycle of CAPI/CAPO and cert-manager. This commit introduces the CAPI
Operator with manifests to declaratively manage these components
instead.

Signed-off-by: Lennart Jern <[email protected]>
@lentzi90 lentzi90 force-pushed the lentzi90/prow-capi-operator branch from e860251 to 354fbba Compare November 26, 2024 13:51
@tuminoid
Copy link
Member

/hold I want to make sure everyone is fine with the way this works before we merge. Changes have not been applied yet.

This is a stepping stone to move Prow management to GitOps, not arguing against. 👏

Copy link
Member

@kashifest kashifest left a comment

Choose a reason for hiding this comment

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

awesome
/approve

@metal3-io-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 26, 2024
Comment on lines +370 to +373
kubectl apply -k capi
# NOTE! You WILL need to apply multiple times before it is successful.
# This is because CRDs and webhooks must be in place before other
# resources can be added.
Copy link
Member Author

Choose a reason for hiding this comment

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

Note this specifically!
This is an opinionated way of working. I put all of it in one kustomization to keep things simple.
However, that kustomization can never be applied in one go. It will always fail the first time you apply, because you cannot create a Certificate or CoreProvider until the CRDs and webhooks are in place.
So the question is, would you rather have a script that does this or is it good like this? Or do you want something different?

To make it crystal clear, the way I would use this if I have to deploy prow from scratch is like this:

  1. kind create cluster
  2. kubectl apply -k capi <- This would fail
  3. Wait a few seconds
  4. kubectl apply -k capi <- This could potentially succeed but will probably fail
  5. Wait a few seconds
  6. kubectl apply -k capi <- This would most likely succeed
  7. Continue to the next step

Copy link
Member

Choose a reason for hiding this comment

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

That is not something you do all the time, so documenting it clearly (like above comment) is enough IMO.

@lentzi90
Copy link
Member Author

/override metal3-ubuntu-e2e-integration-test-main

@metal3-io-bot
Copy link
Collaborator

@lentzi90: Overrode contexts on behalf of lentzi90: metal3-ubuntu-e2e-integration-test-main

In response to this:

/override metal3-ubuntu-e2e-integration-test-main

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 27, 2024
@adilGhaffarDev
Copy link
Member

LGTM. it looks great.

@lentzi90
Copy link
Member Author

/hold cancel

@metal3-io-bot metal3-io-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 27, 2024
@metal3-io-bot metal3-io-bot merged commit 9f02ff7 into metal3-io:main Nov 27, 2024
8 checks passed
@metal3-io-bot metal3-io-bot deleted the lentzi90/prow-capi-operator branch November 27, 2024 08:44
@lentzi90
Copy link
Member Author

(Changes have been applied)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prow: Migrate to CAPI operator
5 participants