✨ Adds IPAM support for floating ips in OpenStackMachine #1762

bilbobrovall · 2023-11-29T13:45:14Z

What this PR does / why we need it:
Implements IPAM support for floating ips in OpenStackMachine

First part of #1750

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
if necessary:
- includes documentation
- adds unit tests

/hold

linux-foundation-easycla · 2023-11-29T13:45:20Z

The committers listed above are authorized under a signed CLA.

✅ login: bilbobrovall / name: Bilbo Brovall (7dce82c)

netlify · 2023-11-29T13:45:21Z

✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name	Link
🔨 Latest commit	`7dce82c`
🔍 Latest deploy log	https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/65f04ca7c791070008c27409
😎 Deploy Preview	https://deploy-preview-1762--kubernetes-sigs-cluster-api-openstack.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

k8s-ci-robot · 2023-11-29T13:45:24Z

Welcome @bilbobrovall!

It looks like this is your first PR to kubernetes-sigs/cluster-api-provider-openstack 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api-provider-openstack has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2023-11-29T13:45:25Z

Hi @bilbobrovall. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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/test-infra repository.

mdbooth · 2023-11-30T16:46:51Z

/check-cla

jichenjc · 2023-12-01T09:07:24Z

/ok-to-test

pkg/utils/names/names.go

jichenjc · 2023-12-01T09:39:39Z

controllers/openstackmachine_controller.go

+}
+
+func (r *OpenStackMachineReconciler) reconcileDeleteFloatingAddressFromPool(scope scope.Scope, openStackMachine *infrav1.OpenStackMachine) error {
+	scope.Logger().Info("Reconciling floating IP claims delete")


I think better to add more info e.g the machine's name in log

controllers/openstackmachine_controller.go

huxcrux · 2023-12-06T10:41:12Z

/retest

huxcrux · 2023-12-11T15:00:36Z

/retest

bilbobrovall · 2023-12-12T07:54:46Z

/retest

dulek

Bunch of questions inline.

controllers/openstackmachine_controller.go

MaysaMacedo

Nice work

config/rbac/role.yaml

controllers/openstackmachine_controller.go

bilbobrovall · 2024-01-24T13:32:21Z

/retest

huxcrux · 2024-03-08T13:05:59Z

I think it is fine to add examples, and even better if we point out that the IPPool is still v1alpha1 🙂

I'll added som basic info to the book, maybe you could take a look and se if you think it's enough?

mdbooth

Critical changes:

Update the missed json tag
Remove status field (or explain why we need it)

With that I think I'd merge this.

mdbooth · 2024-03-08T17:21:43Z

docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md

@@ -104,6 +105,23 @@ The following fields in `PortOpts` are renamed in order to keep them consistent
 * `hostId` becomes `hostID`
 * `allowedCidrs` becomes `allowedCIDRs`

+#### Additon of floatingIPPoolRef


mdbooth · 2024-03-08T17:22:12Z

docs/book/src/topics/crd-changes/v1alpha7-to-v1beta1.md

+#### Additon of floatingIPPoolRef
+
+A new field, FloatingIPPoolRef, has been introduced. It is important to note that this feature requires the existence of an IPPool to operate seamlessly. This new field references an IPPool whice will be used for floating IP allocation.
+In additon to this field an IPPool resource called `OpenStackFloatingIPPool` has been added. Please not that this resource is new and still in alpha meaning its more likely to contain bugs and change in the future.


mdbooth · 2024-03-08T17:25:14Z

api/v1beta1/openstackmachine_types.go

@@ -95,7 +95,7 @@ type OpenStackMachineSpec struct {
 	// to an IPAddressClaim. Once the IPAddressClaim is fulfilled, the FloatingIP
 	// will be assigned to the OpenStackMachine.
 	// +optional
-	FloatingAddressFromPoolRef *corev1.TypedLocalObjectReference `json:"floatingAddressFromPool,omitempty"`
+	FloatingIPPoolRef *corev1.TypedLocalObjectReference `json:"floatingAddressFromPool,omitempty"`


Need to update the json tag.

mdbooth · 2024-03-08T17:49:58Z

controllers/openstackmachine_controller.go

@@ -452,11 +611,32 @@ func (r *OpenStackMachineReconciler) reconcileNormal(ctx context.Context, scope
 		return ctrl.Result{}, nil
 	}

+	err = r.reconcileAPIServerLoadBalancer(scope, openStackCluster, openStackMachine, instanceStatus, instanceNS, clusterName)


Guessing this was to make gocyclo happy?

Yep, initially it was possible to pass by just keeping down additions to reconcileNormal but after having to remove a couple of lines after every rebase, I just gave myself some margins 🫣

mdbooth · 2024-03-08T17:51:19Z

api/v1beta1/openstackmachine_types.go

@@ -107,6 +114,10 @@ type OpenStackMachineStatus struct {
 	// +optional
 	Ready bool `json:"ready"`

+	// FloatingAddressFromPoolReady is true when the FloatingIP is ready.
+	// +optional
+	FloatingAddressFromPoolReady *bool `json:"floatingAddressFromPoolReady,omitempty"`


I don't think we need this because we set a Condition. Unless I've missed something important, please could you remove it?

bilbobrovall · 2024-03-08T22:32:02Z

/hold cancel

bilbobrovall · 2024-03-11T09:03:00Z

Critical changes:

Update the missed json tag

Remove status field (or explain why we need it)

With that I think I'd merge this.

Should be fixed now

dulek · 2024-03-15T15:50:33Z

/lgtm

k8s-ci-robot · 2024-03-15T16:43:05Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bilbobrovall, mdbooth

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:

~~OWNERS~~ [mdbooth]

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 added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 29, 2023

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Nov 29, 2023

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Nov 29, 2023

k8s-ci-robot requested review from jichenjc and mdbooth November 29, 2023 13:45

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 29, 2023

bilbobrovall mentioned this pull request Nov 29, 2023

✨ IPAM provider for floating ips #1763

Merged

3 tasks

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 1, 2023

jichenjc reviewed Dec 1, 2023

View reviewed changes

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 3, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2023

bilbobrovall force-pushed the floating_ip_from_pool branch from 500dd98 to ef85850 Compare December 11, 2023 12:17

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 11, 2023

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 15, 2023

dulek suggested changes Dec 15, 2023

View reviewed changes

MaysaMacedo reviewed Jan 2, 2024

View reviewed changes

config/rbac/role.yaml Outdated Show resolved Hide resolved

controllers/openstackmachine_controller.go Show resolved Hide resolved

bilbobrovall force-pushed the floating_ip_from_pool branch from ab6bb87 to 41f26c1 Compare January 24, 2024 12:22

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2024

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 8, 2024

huxcrux force-pushed the floating_ip_from_pool branch from f25f808 to 9418c5f Compare March 8, 2024 12:52

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 8, 2024

mdbooth reviewed Mar 8, 2024

View reviewed changes

bilbobrovall force-pushed the floating_ip_from_pool branch from 9418c5f to 4dd4578 Compare March 8, 2024 21:11

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 8, 2024

bilbobrovall force-pushed the floating_ip_from_pool branch from 4dd4578 to 51215e6 Compare March 8, 2024 21:30

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 8, 2024

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 8, 2024

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 11, 2024

Get floating IP for openstack machine from IPAM provider

7dce82c

bilbobrovall force-pushed the floating_ip_from_pool branch from 51215e6 to 7dce82c Compare March 12, 2024 12:37

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 12, 2024

k8s-ci-robot assigned dulek Mar 15, 2024

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

mdbooth approved these changes Mar 15, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 15, 2024

k8s-ci-robot merged commit f6fb753 into kubernetes-sigs:main Mar 15, 2024
9 checks passed

huxcrux deleted the floating_ip_from_pool branch March 20, 2024 19:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Adds IPAM support for floating ips in OpenStackMachine #1762

✨ Adds IPAM support for floating ips in OpenStackMachine #1762

bilbobrovall commented Nov 29, 2023 •

edited

linux-foundation-easycla bot commented Nov 29, 2023 •

edited

netlify bot commented Nov 29, 2023 •

edited

k8s-ci-robot commented Nov 29, 2023

k8s-ci-robot commented Nov 29, 2023

mdbooth commented Nov 30, 2023

jichenjc commented Dec 1, 2023

jichenjc Dec 1, 2023

huxcrux commented Dec 6, 2023

huxcrux commented Dec 11, 2023

bilbobrovall commented Dec 12, 2023

dulek left a comment

MaysaMacedo left a comment

bilbobrovall commented Jan 24, 2024

huxcrux commented Mar 8, 2024

mdbooth left a comment

mdbooth Mar 8, 2024

mdbooth Mar 8, 2024

mdbooth Mar 8, 2024

mdbooth Mar 8, 2024

bilbobrovall Mar 8, 2024

mdbooth Mar 8, 2024

bilbobrovall commented Mar 8, 2024

bilbobrovall commented Mar 11, 2024

dulek commented Mar 15, 2024

k8s-ci-robot commented Mar 15, 2024

✨ Adds IPAM support for floating ips in OpenStackMachine #1762

✨ Adds IPAM support for floating ips in OpenStackMachine #1762

Conversation

bilbobrovall commented Nov 29, 2023 • edited

linux-foundation-easycla bot commented Nov 29, 2023 • edited

netlify bot commented Nov 29, 2023 • edited

✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

k8s-ci-robot commented Nov 29, 2023

k8s-ci-robot commented Nov 29, 2023

mdbooth commented Nov 30, 2023

jichenjc commented Dec 1, 2023

jichenjc Dec 1, 2023

Choose a reason for hiding this comment

huxcrux commented Dec 6, 2023

huxcrux commented Dec 11, 2023

bilbobrovall commented Dec 12, 2023

dulek left a comment

Choose a reason for hiding this comment

MaysaMacedo left a comment

Choose a reason for hiding this comment

bilbobrovall commented Jan 24, 2024

huxcrux commented Mar 8, 2024

mdbooth left a comment

Choose a reason for hiding this comment

mdbooth Mar 8, 2024

Choose a reason for hiding this comment

mdbooth Mar 8, 2024

Choose a reason for hiding this comment

mdbooth Mar 8, 2024

Choose a reason for hiding this comment

mdbooth Mar 8, 2024

Choose a reason for hiding this comment

bilbobrovall Mar 8, 2024

Choose a reason for hiding this comment

mdbooth Mar 8, 2024

Choose a reason for hiding this comment

bilbobrovall commented Mar 8, 2024

bilbobrovall commented Mar 11, 2024

dulek commented Mar 15, 2024

k8s-ci-robot commented Mar 15, 2024

bilbobrovall commented Nov 29, 2023 •

edited

linux-foundation-easycla bot commented Nov 29, 2023 •

edited

netlify bot commented Nov 29, 2023 •

edited