Repository: kubermatic/machine-controller
Pull request #1753 (2024-01-03 11:42:01)

Anexia: Adds field to check if a IP reservation is still valid
**What this PR does / why we need it**:
Adds a field to check if a IP reservation is still valid, if the IP is no longer valid a new one is requested.

This fixes a bug where we use a IP reservation that expired already for a VM, if the VM provisioning failed, which then leads to a race condition. In which we wait for the VM provisioning to finish, but it can't due the IP being no longer useable, but we also never request a new IP in that case.

**What type of PR is this?**
/kind bug
<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
/kind chore
-->

**Special notes for your reviewer**:
Please backport to version v1.57

**Does this PR introduce a user-facing change? Then add your Release Note here**:
<!--
Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
Anexia: Fix IP reservation related race condition.
```

**Documentation**:
<!--
Please do one of the following options:
- Add a link to the existing documentation
- Add a link to the kubermatic/docs pull request
- If no documentation change is applicable then add:
  - TBD (documentation will be added later)
  - NONE (no documentation needed for this PR)
-->
```documentation
NONE
```


kubermatic-bot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
**Once this PR has been reviewed and has the lgtm label**, please assign [kron4eg](https://github.com/kron4eg) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubermatic/machine-controller/blob/main/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["kron4eg"]} -->
kubermatic-bot: Hi @89Q12. Thanks for your PR.

I'm waiting for a [kubermatic](https://github.com/orgs/kubermatic/people) 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](https://github.com/orgs/kubermatic/people) 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](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

kubermatic-bot: LGTM label has been added.  <details>Git tree hash: 75e4bf9fa15f359e7b9f1b2e823d80139031a945</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/1753#pullrequestreview-1805561738" title="Approved">ahmedwaleedmalik</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/main/OWNERS)~~ [ahmedwaleedmalik]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
kubermatic-bot: @89Q12: The following test **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-machine-controller-e2e-aws-spot-instance | 4f2f9dee5e94747a7901e5b91fb1845fa970f32d | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1753/pull-machine-controller-e2e-aws-spot-instance/1743193797887528960) | true | `/test pull-machine-controller-e2e-aws-spot-instance`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=1753)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: @89Q12: The following tests **failed**, say `/retest` to rerun all failed tests or `/retest-required` to rerun all mandatory failed tests:

Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
pull-machine-controller-e2e-aws-spot-instance | 4f2f9dee5e94747a7901e5b91fb1845fa970f32d | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1753/pull-machine-controller-e2e-aws-spot-instance/1743193797887528960) | true | `/test pull-machine-controller-e2e-aws-spot-instance`
pull-machine-controller-e2e-aws-spot-instance-legacy-userdata | 4f2f9dee5e94747a7901e5b91fb1845fa970f32d | [link](https://public-prow.loodse.com/view/gs/prow-dev-public-data/pr-logs/pull/kubermatic_machine-controller/1753/pull-machine-controller-e2e-aws-spot-instance-legacy-userdata/1743193797820420096) | true | `/test pull-machine-controller-e2e-aws-spot-instance-legacy-userdata`

[Full PR test history](https://public-prow.loodse.com/pr-history?org=kubermatic&repo=machine-controller&pr=1753)

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
kubermatic-bot: New changes are detected. LGTM label has been removed.
kubermatic-bot: LGTM label has been added.  <details>Git tree hash: 3d2f84e992e511c8d3522c86d2e3e8614787e76a</details>
kubermatic-bot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubermatic/machine-controller/pull/1753#pullrequestreview-1805785300" title="Approved">ahmedwaleedmalik</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubermatic%2Fmachine-controller).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/kubermatic/machine-controller/blob/main/OWNERS)~~ [ahmedwaleedmalik]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/pkg/cloudprovider/provider/anexia/provider.go b/pkg/cloudprovider/provider/anexia/provider.go
index e381a388..c861e1ee 100644
--- a/pkg/cloudprovider/provider/anexia/provider.go
+++ b/pkg/cloudprovider/provider/anexia/provider.go
@@ -233,7 +233,7 @@ func getIPAddress(ctx context.Context, log *zap.SugaredLogger, client anxclient.
 	status := reconcileContext.Status
 
 	// only use IP if it is still unbound
-	if status.ReservedIP != "" && status.IPState == anxtypes.IPStateUnbound {
+	if status.ReservedIP != "" && status.IPState == anxtypes.IPStateUnbound && (!status.IPProvisioningExpires.IsZero() && status.IPProvisioningExpires.After(time.Now())) {
 		log.Infow("Re-using already provisioned IP", "ip", status.ReservedIP)
 		return status.ReservedIP, nil
 	}
@@ -259,6 +259,7 @@ func getIPAddress(ctx context.Context, log *zap.SugaredLogger, client anxclient.
 	ip := res.Data[0].Address
 	status.ReservedIP = ip
 	status.IPState = anxtypes.IPStateUnbound
+	status.IPProvisioningExpires = time.Now().Add(anxtypes.IPProvisioningExpires)
 
 	return ip, nil
 }
diff --git a/pkg/cloudprovider/provider/anexia/types/types.go b/pkg/cloudprovider/provider/anexia/types/types.go
index 3686defe..f97a5389 100644
--- a/pkg/cloudprovider/provider/anexia/types/types.go
+++ b/pkg/cloudprovider/provider/anexia/types/types.go
@@ -34,8 +34,9 @@ const (
 	GetRequestTimeout    = 1 * time.Minute
 	DeleteRequestTimeout = 1 * time.Minute
 
-	IPStateBound   = "Bound"
-	IPStateUnbound = "Unbound"
+	IPStateBound          = "Bound"
+	IPStateUnbound        = "Unbound"
+	IPProvisioningExpires = 1800 * time.Second
 
 	VmxNet3NIC       = "vmxnet3"
 	MachinePoweredOn = "poweredOn"
@@ -72,12 +73,13 @@ type RawConfig struct {
 }
 
 type ProviderStatus struct {
-	InstanceID       string         `json:"instanceID"`
-	ProvisioningID   string         `json:"provisioningID"`
-	DeprovisioningID string         `json:"deprovisioningID"`
-	ReservedIP       string         `json:"reservedIP"`
-	IPState          string         `json:"ipState"`
-	Conditions       []v1.Condition `json:"conditions,omitempty"`
+	InstanceID            string         `json:"instanceID"`
+	ProvisioningID        string         `json:"provisioningID"`
+	DeprovisioningID      string         `json:"deprovisioningID"`
+	ReservedIP            string         `json:"reservedIP"`
+	IPState               string         `json:"ipState"`
+	IPProvisioningExpires time.Time      `json:"ipProvisioningExpires"`
+	Conditions            []v1.Condition `json:"conditions,omitempty"`
 }
 
 func GetConfig(pconfig providerconfigtypes.Config) (*RawConfig, error) {


Test patch:
diff --git a/pkg/cloudprovider/provider/anexia/provider_test.go b/pkg/cloudprovider/provider/anexia/provider_test.go
index 7754ebb6..376610df 100644
--- a/pkg/cloudprovider/provider/anexia/provider_test.go
+++ b/pkg/cloudprovider/provider/anexia/provider_test.go
@@ -301,6 +301,7 @@ func TestAnexiaProvider(t *testing.T) {
 			expectedIP := "8.8.8.8"
 			providerStatus.ReservedIP = expectedIP
 			providerStatus.IPState = anxtypes.IPStateUnbound
+			providerStatus.IPProvisioningExpires = time.Now().Add(anxtypes.IPProvisioningExpires)
 			reservedIP, err := getIPAddress(ctx, log, client)
 			testhelper.AssertNoErr(t, err)
 			testhelper.AssertEquals(t, expectedIP, reservedIP)