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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

馃尡 clusterctl: add in-cluster ipam provider #8811

Merged
merged 1 commit into from
Feb 19, 2024
Merged

馃尡 clusterctl: add in-cluster ipam provider #8811

merged 1 commit into from
Feb 19, 2024

Conversation

schrej
Copy link
Member

@schrej schrej commented Jun 7, 2023

What this PR does / why we need it:
Adds the in-cluster ipam provider to clusterctl.

Draft until we release version 0.1.0 of the provider.

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. area/clusterctl Issues or PRs related to clusterctl labels Jun 7, 2023
sbueringer
sbueringer reviewed Jul 26, 2023
View reviewed changes
cmd/clusterctl/client/config/providers_client.go
@@ -315,6 +320,13 @@ func (p *providersClient) defaults() []Provider {
providerType: clusterctlv1.ControlPlaneProviderType,
},

// IPAM providers
Copy link
Member

Choose a reason for hiding this comment

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

Let's also add the provider to the glossary etc.

For reference: #9012 (should be ~ the same)

@sbueringer
Copy link
Member

sbueringer commented Nov 16, 2023 •
edited
Loading

@schrej @rikatz Any plans of cutting a v0.1.0 release and adding the provider to clusterctl? :)

@schrej
Copy link
Member Author

schrej commented Nov 30, 2023

@schrej @rikatz Any plans of cutting a v0.1.0 release and adding the provider to clusterctl? :)

Yes. We still need to finish setting up some official image builds. I think we were waiting for a review, I'll take another look.

@fabriziopandini fabriziopandini mentioned this pull request Jan 15, 2024
Closed
@schrej schrej marked this pull request as ready for review February 14, 2024 19:28
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 14, 2024
schrej
schrej commented Feb 15, 2024
View reviewed changes
docs/book/src/reference/glossary.md
@@ -108,6 +108,9 @@ Cluster API Provider VMware Cloud Director
### CAPZ
Cluster API Provider Azure

### CAIPAMIC
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about CAIPAM as a prefix for ipam providers, but probably the best option next to CAIP

Copy link
Member

Choose a reason for hiding this comment

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

Sounds reasonable to me

@schrej
Copy link
Member Author

schrej commented Feb 15, 2024

That failed GH action seems to be a flake, that link is working fine for me.

@sbueringer
Copy link
Member

Yeah saw that Azure link failing at least 10-20x times in the last month

@sbueringer
Copy link
Member

Thank you!

/lgtm

/assign @fabriziopandini @chrischdi

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 15, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: fe96f3e0a66e10486b744aa5eb2743aa3641852e

@guettli
Copy link
Contributor

guettli commented Feb 16, 2024

Yeah saw that Azure link failing at least 10-20x times in the last month

@sbueringer is someone working on that, so that we have stable tests?

@fabriziopandini
Copy link
Member

/lgtm
/approve

馃コ

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 19, 2024
@k8s-ci-robot k8s-ci-robot merged commit 323f8bb into kubernetes-sigs:main Feb 19, 2024
21 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v1.7 milestone Feb 19, 2024
@schrej schrej deleted the clusterctl-ipam-in-cluster branch February 19, 2024 17:31
@sbueringer
Copy link
Member

Yeah saw that Azure link failing at least 10-20x times in the last month

@sbueringer is someone working on that, so that we have stable tests?

Not that I'm aware of

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sbueringer sbueringer sbueringer left review comments

@JoelSpeed JoelSpeed Awaiting requested review from JoelSpeed

@ykakarap ykakarap Awaiting requested review from ykakarap

@richardcase richardcase Awaiting requested review from richardcase

@stmcginnis stmcginnis Awaiting requested review from stmcginnis

Assignees

@chrischdi chrischdi

@sbueringer sbueringer

@fabriziopandini fabriziopandini

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/clusterctl Issues or PRs related to clusterctl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@schrej @k8s-ci-robot @sbueringer @guettli @fabriziopandini @chrischdi