⚠️ Remove redundant fields from CABPK / KCP ClusterConfiguration

[sbueringer]

Signed-off-by: Stefan Büringer [email protected]

What this PR does / why we need it:

The PR drops fields that can be configured by other means and thus don't have to exist redundantly on the ClusterConfiguration struct.

The removed fields are:

• Networking.ServiceSubnet
• Networking.PodSubnet
• Networking.DNSDomain
• KubernetesVersion
• ControlPlaneEndpoint
• ClusterName

WIP for now because it contains #12303

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #10852

[sbueringer]

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-33-1-34-main

[sbueringer]

/test pull-cluster-api-e2e-upgrade-1-33-1-34-main

[sbueringer]

/test pull-cluster-api-e2e-upgrade-1-33-1-34-main

[sbueringer]

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-33-1-34-main

[sbueringer]

/hold
for rebase after #12303 merges.

Only the last commit of this PR will remain (and is ready for review now)

[sbueringer]

/assign @fabriziopandini @chrischdi

[chrischdi]

lgtm for the last commit

[sbueringer]

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-33-1-34-main

[sbueringer]

/assign @fabriziopandini @chrischdi

ready now :)

[chrischdi]

Very nice cleanup! Thanks!

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: a0c5ef216b282171a7e2a87249d0f3e3afa32633

[fabriziopandini]

Wondering if we should give a stronger warning about this change (or generalizing all the changes where we loose info in the v1beta1 (old client)->v1beta2 (storage) -> v1beta1 (old client) e.g.

If you are using git ops tools, either migrate to v1beta2 or make sure to drop following fields to prevent issues:

[sbueringer]

/test pull-cluster-api-e2e-conformance-ci-latest-main
/test pull-cluster-api-e2e-conformance-main
/test pull-cluster-api-e2e-latestk8s-main
/test pull-cluster-api-e2e-main
/test pull-cluster-api-e2e-mink8s-main
/test pull-cluster-api-e2e-upgrade-1-33-1-34-main

[fabriziopandini]

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 712ff84d071d051d01e9bff541d8cdbcf3bf3132

[k8s-ci-robot]

[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:

• OWNERS [fabriziopandini]

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

[Karthik-K-N]

@sbueringer A question regarding setting ControlPlaneEndpoint when using v1beta2 ,

In CAPIBM we use kube-vip and used to set one external IP for control-plane endpoint and internal IP for cluster, The cluster config looks like below

apiVersion: infrastructure.cluster.x-k8s.io/v1beta2
kind: IBMPowerVSCluster
metadata:
  labels:
    cluster.x-k8s.io/cluster-name: ibm-powervs-1
  name: ibm-powervs-1
  namespace: default
spec:
  controlPlaneEndpoint:
    host: 163.68.77.203 // External IP
    port: 6443
  network:
    name: karthik-capi-test
  serviceInstanceID: 10b1000b-da8d-4e18-ad1f-6b2a56a8c130

In kube-vip we use internal ip

apiVersion: controlplane.cluster.x-k8s.io/v1beta2
kind: KubeadmControlPlane
metadata:
  name: ibm-powervs-1-control-plane
  namespace: default
spec:
  kubeadmConfigSpec:
    clusterConfiguration:
      apiServer:
        certSANs:
        - 192.168.169.203
        - 163.68.77.203
        extraArgs:
        - name: cloud-provider
          value: external
      controlPlaneEndpoint: 192.168.169.203:6443 // Internal IP
      controllerManager:
        extraArgs:
        - name: cloud-provider
          value: external
    files:
    - content: |
        apiVersion: v1
        kind: Pod
        metadata:
          creationTimestamp: null
          name: kube-vip
          namespace: kube-system
          .
          .

With this field omission, IIUC the cluster.spec.controlplaneendpoint will be used in kubeadm config spec, So we are not able to use two different IPs. Or is there any other better way to manage this.

[sbueringer]

Wow. I did not know that this is possible

(cc @fabriziopandini)

[mkumatag]

With this field omission, IIUC the cluster.spec.controlplaneendpoint will be used in kubeadm config spec, So we are not able to use two different IPs. Or is there any other better way to manage this.

lets explore if these fields are helpful

cluster-api/api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

Line 97 in 7a22509

LocalAPIEndpoint APIEndpoint `json:"localAPIEndpoint,omitempty"`

cluster-api/api/bootstrap/kubeadm/v1beta2/kubeadm_types.go

Line 539 in 7a22509

LocalAPIEndpoint APIEndpoint `json:"localAPIEndpoint,omitempty"`

[Amulyam24]

lets explore if these fields are helpful
cluster-api/api/bootstrap/kubeadm/v1beta2/kubeadm_types.go
LocalAPIEndpoint APIEndpoint json:"localAPIEndpoint,omitempty"

cluster-api/api/bootstrap/kubeadm/v1beta2/kubeadm_types.go
LocalAPIEndpoint APIEndpoint json:"localAPIEndpoint,omitempty"

It did not work upon configuring with this.

Upon debugging further with @Karthik-K-N, the only work around which worked to get a successful cluster creation was adding a script in preKubeadmCommands to change the controlPlaneEndpoint in /run/kubeadm/kubeadm.yaml

After removing the field from kubeadmConfigSpec. clusterConfiguration, it was set to the external IP which was being taken from IBMPowerVSCluster.spec.controlPlaneEndpoint.

Replacing it with the internal IP which is configured with kube-vip

sed -i 's#controlPlaneEndpoint: {EXTERNAL_IP}:6443#controlPlaneEndpoint: {INTERNAL_IP}:6443#' /run/kubeadm/kubeadm.yaml

@sbueringer, would this be recommended or is there a better way to handle this?

[mkumatag]

It did not work upon configuring with this.

interesting! as per the api doc it should work, may be we need to explore why it is not working?!

[sbueringer]

@mkumatag @Amulyam24 @Karthik-K-N

Hey folks,
thx for the feedback and thx for looking into this. I looked into this with Fabrizio as well. We're basically going to revert the PR for this field (I"ll have a PR open in a bit)

[sbueringer]

@mkumatag @Amulyam24 @Karthik-K-N Please see: #12423