Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Fix a bug in ACT_main json patch for Azure Cluster Class #4622

Open
wants to merge 1 commit into
base: release-0.29
Choose a base branch
from

Conversation

PierrePIRONIN
Copy link

Fix a go template bug which makes invalid the template rendering for the AzureClusterTemplate to inject mulitple VNET cidrs.

What this PR does / why we need it

Fix a bug which blocks build of Azure Kubernetes cluster with multiple VNET cidrs (see related issue for more details).

Which issue(s) this PR fixes

Fixes #4621

Describe testing done for PR

Created Azure workload cluster to verify change and it works.

Release note

Fix a go template bug which makes invalid the template rendering for the AzureClusterTemplate to inject mulitple VNET cidrs.

Additional information

Special notes for your reviewer

I’ve worked on it jointly with Pablo Iglesias (VMware Customer 0 team member)

Fix a go template bug which makes invalid the template rendering for the AzureClusterTemplate to inject mulitple VNET cidrs.

Here is the capi-controller error log message without this fix:
E0525 14:28:24.579774       1 controller.go:326] "Reconciler error" err="error reconciling the Cluster topology: failed to create AzureCluster.infrastructure.cluster.x-k8s.io: FieldValueInvalid: spec.networkSpec.cidrBlocks: Invalid value: \"10.221.98.192/27- 10.221.98.224/27\": invalid CIDR format FieldValueInvalid: spec.networkSpec.subnets[0].cidrBlocks: Invalid value: \"10.221.98.192/27\": subnet CIDR not in vnet address space: [10.221.98.192/27- 10.221.98.224/27] FieldValueInvalid: spec.networkSpec.subnets[1].cidrBlocks: Invalid value: \"10.221.98.224/27\": subnet CIDR not in vnet address space: [10.221.98.192/27- 10.221.98.224/27]" controller="topology/cluster" controllerGroup="cluster.x-k8s.io" controllerKind="Cluster" cluster="k8s-mma/k8s-mma" namespace="k8s-mma" name="k8s-mma" reconcileID=eea64084-273f-4028-9b70-374514886761
@PierrePIRONIN PierrePIRONIN requested review from a team as code owners May 25, 2023 15:05
@vmwclabot
Copy link

@PierrePIRONIN, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@github-actions
Copy link

Hi @PierrePIRONIN! And thank you for opening your first Pull Request. Someone will review it soon. Thank you for committing to making Tanzu Framework better.

@vmwclabot
Copy link

@PierrePIRONIN, we have received your signed contributor license agreement. The review is usually completed within a week, but may take longer under certain circumstances. Another comment will be added to the pull request to notify you when the merge can proceed.

@vmwclabot
Copy link

@PierrePIRONIN, VMware has approved your signed contributor license agreement.

@yastij
Copy link
Member

yastij commented Jun 7, 2023

@DanielXiao - can you take a look ?

@@ -759,7 +759,7 @@ spec:
name: {{ if .network.vnet.name }} {{- .network.vnet.name }} {{- else }} {{- .builtin.cluster.name -}} -vnet {{- end }}
resourceGroup: {{ if .network.vnet.resourceGroup }} {{- .network.vnet.resourceGroup }} {{- else if .resourceGroup }} {{- .resourceGroup }} {{- else }} {{- .builtin.cluster.name }} {{- end }}
cidrBlocks:
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems there will be an empty line, we usually write iteration like this

            cidrBlocks:
            {{- range .network.vnet.cidrBlocks }}
            - {{ . }}
            {{- end }}

Copy link
Member

@joshuatcasey joshuatcasey left a comment

Choose a reason for hiding this comment

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

Approving for @tkg-iam-owners

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants