✨Add clusterctl options to show templates and cluster resource sets

[Jont828]

What this PR does / why we need it: This PR adds additional options to the Discovery client for clusterctl describe. These changes are meant to (1) allow the command to show all resources associated with the cluster and (2) allow the client to construct a tree that can be used as an input for a separate Cluster API GUI.

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

[k8s-ci-robot]

Hi @Jont828. 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.

[sbueringer]

/ok-to-test

[Jont828]

Actually we can hold this for now, I'm still trying to see if I can add some other features and also clean up some of the code.

/hold

[fabriziopandini]

@ykakarap

[ykakarap]

/cc

[fabriziopandini]

Given this is still WIP, feel free to ignore comments

[fabriziopandini]

A couple of comments, mostly for

• better understanding the use case you are trying to address
• avoid mixing semantics of flags/flags with semantics too generic
• avoid to load too many objects at T0 (we can consider incremental loading if it makes sense)

[Jont828]

@fabriziopandini Do you think we'd want to add a placeholder unstructured object in this case as well?

[fabriziopandini]

might be in a followUp PR, let's keep this one scoped (it is already adding a bunch of flag in the same changeset)

[Jont828]

We could probably add a "ShowMachinePools" flag similar to "ShowMachineSets" and replicate the behavior once MachinePoolMachines are added.

[Jont828]

Actually we might not need that at all since we have --echo and --grouping=false already

[killianmuldoon]

Are there any assumptions that we can make about the name/location of the infrastructureRef/infrastructureTemplate in control planes? While the contents of unstructured.Unstructured types are being used for provider specific info, I think it's still useful to include the machine template for the control plane in the output.

If I get you the assumptions you can make are part of the contract e.g. for spec fields: https://cluster-api.sigs.k8s.io/developer/architecture/controllers/control-plane.html#required-spec-fields-for-implementations-using-machines. So you should have machineTemplate.infrastructureRef

e.g. a control plane on my dev cluster has the spec:

  spec:
    kubeadmConfigSpec:
      clusterConfiguration:
        apiServer:
          certSANs:
          - localhost
          - 127.0.0.1
          - 0.0.0.0
        controllerManager:
          extraArgs:
            enable-hostpath-provisioner: "true"
        dns: {}
        etcd:
          local: {}
        imageRepository: k8s.gcr.io
        networking: {}
        scheduler: {}
      format: cloud-config
      initConfiguration:
        localAPIEndpoint: {}
        nodeRegistration:
          criSocket: unix:///var/run/containerd/containerd.sock
          kubeletExtraArgs:
            cgroup-driver: cgroupfs
            eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%
      joinConfiguration:
        discovery: {}
        nodeRegistration:
          criSocket: unix:///var/run/containerd/containerd.sock
          kubeletExtraArgs:
            cgroup-driver: cgroupfs
            eviction-hard: nodefs.available<0%,nodefs.inodesFree<0%,imagefs.available<0%
    machineTemplate:
      infrastructureRef:
        apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
        kind: DockerMachineTemplate
        name: cloister-control-plane-lmbll
        namespace: default
      metadata:
        labels:
          cluster.x-k8s.io/cluster-name: cloister
          topology.cluster.x-k8s.io/owned: ""
    replicas: 3
    rolloutStrategy:
      rollingUpdate:
        maxSurge: 1
      type: RollingUpdate
    version: v1.21.2

[killianmuldoon]

@Jont828 I think the spec.infrastructureTemplate variant is the way things were with the v1alpha3 API. I just created a PR #6392 to update the example in the book - did you see that structure used somewhere else? Maybe we need an update in clusterctl code too.

[Jont828]

@killianmuldoon Yeah I'm seeing the spec.infrastructureTemplate field in internal fake types here. In that case is it safe to assume that we will update the fake control plane type?

[ykakarap]

@ykakarap @fabriziopandini So I looked into adding tests but it seems like I found an small issue. Kubeadmcontrolplanes have the infrastructureRef field in spec.machineTemplate.infrastructureRef while our fake controlPlane has the field in spec.infrastuctureTemplate.

Are there any assumptions that we can make about the name/location of the infrastructureRef/infrastructureTemplate in control planes? While the contents of unstructured.Unstructured types are being used for provider specific info, I think it's still useful to include the machine template for the control plane in the output.

The infrastructureRef path is different in the real and the GenericControlPlane type. GenericControlPlane most likely is structured like that for historical reasons to match with v1alpha3 types. Changing GenericControlPlaneSpec to :

// GenericMachineTemplate contains the generic machine template.
type GenericMachineTemplate struct {
	InfrastructureRef corev1.ObjectReference `json:"infrastructureRef"`
}

// GenericControlPlaneSpec contains a generic control plane spec.
type GenericControlPlaneSpec struct {
	MachineTemplate GenericMachineTemplate `json:"machineTemplate"`
}

should be fine. I believe it will not affect any of the other tests (I have tested this locally) as all others tests that use GenericControlPlane primarily look for values in ownerReferences and will not be affected by this path.

PS: You will have to run make generate-go-deepcopy-core after making the change to generate deep copy functions for the modified type.

[Jont828]

@ykakarap Sounds good, I wrote some tests, including the edge case you found with having two machine deployments. I tagged Fabrizio in a minor comment about the test cases, but otherwise I think we should be good for a final review pass.

[fabriziopandini]

I have tested this locally and it works great!
please fix lint errors and remove the hold, IMO we are ready to merge!

[fabriziopandini]

/lgtm
/hold cancel
waiting for the last check from @ykakarap if he can get to it

[ykakarap]

Tested locally and everything seems to bo working as expected.

This is great work @Jont828 🚀
Thanks for doing this!

Just small nits (non-blocking).

/lgtm

[Jont828]

Just pushed some changes to fix the nits. Huge shoutout to @fabriziopandini for tons of feedback and design suggestions!

[ykakarap]

/lgtm

[fabriziopandini]

/lgtm
/approve
Yay!

[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