OCPBUGS-56104,OCPCLOUD-2893: Add related objects to must-gather config

[honza]

No description provided.

[openshift-ci-robot]

@honza: This pull request references OCPCLOUD-2893 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

Details

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

[nrb]

/assign @damdo

[nrb]

I think as a first pass this is good - ideally, we'd use discovery to grab all CRDs in the group, but I think that at the moment it's more important to have the functionality than to spend time on the generator.

Also, the resources should be plural; hopefully you can just accept the suggestions to have it done automatically.

[honza]

ideally, we'd use discovery to grab all CRDs in the group,

Is there any prior art on this? I looked but haven't found anything. I have a WIP uncommitted code that tries to do this but it feels wrong.

[nrb]

Is there any prior art on this?

Not that I can find; I was hoping so, but perhaps it doesn't exist on purpose.

it feels wrong.

Why's that?

[honza]

func (r *ClusterOperatorStatusClient) relatedObjects() []configv1.ObjectReference {
	// TBD: Add an actual set of object references from getResources method
	infra, err := util.GetInfra(context.TODO(), r.Client)
	if err != nil {
		// TODO
	}
	platform, err := util.GetPlatform(context.TODO(), infra)
	if err != nil {
		// TODO
	}

	m := r.Scheme().AllKnownTypes()

	for groupVersionKind, t := range m {

		if strings.Contains(groupVersionKind.Kind, "Options") {
			continue
		}
		if groupVersionKind.Kind == "WatchEvent" {
			continue
		}
		if strings.Contains(groupVersionKind.Group, "cluster.x-k8s.io") {
			field, found := t.FieldByName("ObjectMeta")
			if !found {
				continue
			}

			// TODO: match based on platform ^^^
		}
	}

	return []configv1.ObjectReference{
		{Resource: "namespaces", Name: controllers.DefaultManagedNamespace},
		{Group: configv1.GroupName, Resource: "clusteroperators", Name: controllers.ClusterOperatorName},
		{Resource: "namespaces", Name: r.ManagedNamespace},
		{Group: "", Resource: "serviceaccounts", Name: "cluster-capi-operator", Namespace: controllers.DefaultManagedNamespace},
		{Group: "", Resource: "configmaps", Name: "cluster-capi-operator-images", Namespace: controllers.DefaultManagedNamespace},
		{Group: "apps", Resource: "deployments", Name: "cluster-capi-operator", Namespace: controllers.DefaultManagedNamespace},
		{Group: "cluster.x-k8s.io", Resource: "clusters", Namespace: controllers.DefaultManagedNamespace},
		{Group: "cluster.x-k8s.io", Resource: "machines", Namespace: controllers.DefaultManagedNamespace},
	}
}

[damdo]

Hey @honza thanks for this PR.

Looking at the gather-extra from one of the e2e jobs I only see machines/machinesets, and not for example core CAPI cluster.

From the classic must-gather instead I also see cluster, but I still don't see awscluster for example.

Any ideas on why?

[theobarberbany]

👋 Reviving this, as having this functionality in must-gathers would be awesome.

Is there anything I can do to help?

@honza, I think your code in the above comment looks ok as a starting point. If we can get it commited we can review? :)

[honza]

Let me clean it up and push.

[honza]

In the classic gather, we now have:

$ tree namespaces/openshift-cluster-api/infrastructure.cluster.x-k8s.io/
infrastructure.cluster.x-k8s.io/
├── awsclusters
│   └── ci-op-yi9j3qyc-c3c99-58vxn.yaml
├── awsmachines
│   ├── ci-op-yi9j3qyc-c3c99-58vxn-worker-us-east-2b-7r2bh.yaml
│   ├── ci-op-yi9j3qyc-c3c99-58vxn-worker-us-east-2b-fk8k8.yaml
│   └── ci-op-yi9j3qyc-c3c99-58vxn-worker-us-east-2c-2qrcx.yaml
└── awsmachinetemplates
    ├── ci-op-yi9j3qyc-c3c99-58vxn-worker-us-east-2b.yaml
    └── ci-op-yi9j3qyc-c3c99-58vxn-worker-us-east-2c.yaml

4 directories, 6 files

[honza]

I think the extra gather commands are here. We could add a big switch statement and collect the infra resources based on platform.

[damdo]

I think the extra gather commands are here. We could add a big switch statement and collect the infra resources based on platform.

Yeah we could 👍

[honza]

openshift/release#64484

[damdo]

@honza yes openshift/release#56322 covers the gather-extra, but for the must-gather, the change was missing. Thanks for adding it, let's confirm this is in the must-gather artifact before proceeding with the merge.

[wking]

If you're actively managing these entries in your operator (which it looks like you are), I don't think you need to go through the work of manually listing them here in your ClusterOperator manifest. You need enough in the manifest that a must-gather while your operator isn't running collects enough to figure out why the operator isn't running. And it seems unlikely that folks would need to get all the way out to cloud-specific types to figure out why the operator wasn't running?

But also, 🤷, if you don't mind managing this list by hand or you find some way to automate it, having the cloud-specific types in this manifest doesn't hurt.

[honza]

$ tree namespaces/openshift-cluster-api/cluster.x-k8s.io/machinesets/
namespaces/openshift-cluster-api/cluster.x-k8s.io/machinesets/
├── ci-op-hq03lh3v-c3c99-2nvtb-worker-us-east-2b.yaml
└── ci-op-hq03lh3v-c3c99-2nvtb-worker-us-east-2c.yaml

1 directory, 2 files

[honza]

I also noticed this in the Azure job:

$ tree infrastructure.cluster.x-k8s.io
infrastructure.cluster.x-k8s.io
├── azureclusteridentities
│   └── ci-op-hq03lh3v-d9d1a-g4jn8.yaml
└── azureclusters
    └── ci-op-hq03lh3v-d9d1a-g4jn8.yaml

3 directories, 2 files

[honza]

vsphere is the same

My theory is that the e2e test deletes those resources before we gather. Does that sound right?

[damdo]

I also noticed this in the Azure job:

$ tree infrastructure.cluster.x-k8s.io
infrastructure.cluster.x-k8s.io
├── azureclusteridentities
│   └── ci-op-hq03lh3v-d9d1a-g4jn8.yaml
└── azureclusters
    └── ci-op-hq03lh3v-d9d1a-g4jn8.yaml

3 directories, 2 files

Yes in azure we create an azureclusteridentity in the infracluster controller, as that's required there.
But we only do it for Azure.

cluster-capi-operator/pkg/controllers/infracluster/azure.go

Lines 178 to 200 in ac5aa33

	azureClusterIdentity := &azurev1.AzureClusterIdentity{
	ObjectMeta: metav1.ObjectMeta{
	Name: r.Infra.Status.InfrastructureName,
	Namespace: defaultCAPINamespace,
	Annotations: map[string]string{
	// The ManagedBy Annotation is set so CAPI infra providers ignore the InfraCluster object,
	// as that's managed externally, in this case by the cluster-capi-operator's infracluster controller.
	clusterv1.ManagedByAnnotation: managedByAnnotationValueClusterCAPIOperatorInfraClusterController,
	},
	},
	Spec: azurev1.AzureClusterIdentitySpec{
	Type: azurev1.ServicePrincipal,
	AllowedNamespaces: &azurev1.AllowedNamespaces{NamespaceList: []string{defaultCAPINamespace}},
	ClientID: string(azureClientID),
	TenantID: string(azureTenantID),
	ClientSecret: corev1.SecretReference{Name: clusterSecretName, Namespace: defaultCAPINamespace},
	},
	}
	// The Azure Cluster Identtiy does not exist, so it needs to be created.
	if err := r.Create(ctx, azureClusterIdentity); err != nil && !cerrors.IsAlreadyExists(err) {
	return fmt.Errorf("failed to create Azure Cluster Identity: %w", err)
	}

---

vsphere is the same

My theory is that the e2e test deletes those resources before we gather. Does that sound right?

@honza What do you mean by "vsphere is the same"?
I didn't notice anything peculiar in the vpshere gathering:

[~/Downloads/must-gather (6)/registry-apps-build02-vmc-ci-openshift-org-ci-op-hq03lh3v-stable-sha256-72b6d67be00f7b3b2f2e00ac24632b102079b7d2c4f4f1effd38295b199fa19f] $ tree -L 2 namespaces/openshift-cluster-api/infrastructure.cluster.x-k8s.io/
namespaces/openshift-cluster-api/infrastructure.cluster.x-k8s.io/
└── vsphereclusters
    └── ci-op-hq03lh3v-6e495-d7vzg.yaml

2 directories, 1 file

[honza]

What do you mean by "vsphere is the same"? I didn't notice anything peculiar in the vpshere gathering

I was distracted by the missing infra machines in the gather. It looks like we're good to go?

[damdo]

Yeah at the moment I would only expect AWS to still have machines at the end of the e2e-capi-techpreview test when the must-gather runs.

The e2e's for all of the other platforms do clean up the machines/machinesets they create during the test before considering it passed.

[damdo]

/unhold

/lgtm

[damdo]

Thanks for working on this @honza 🎉

[honza]

/test e2e-aws-ovn-serial

[damdo]

The serial job passed but timed out on tear-down hitting the 4h mark: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-capi-operator/267/pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-serial/1920845242664226816

I am happy to override it

/override ci/prow/e2e-aws-ovn-serial

[openshift-ci]

@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-aws-ovn-serial

Details

In response to this:

The serial job passed but timed out on tear-down hitting the 4h mark: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-capi-operator/267/pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-serial/1920845242664226816

I am happy to override it

/override ci/prow/e2e-aws-ovn-serial

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-sigs/prow repository.

[damdo]

/retest

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD d131690 and 2 for PR HEAD 706d6f9 in total

[damdo]

The serial job passed but timed out on tear-down hitting the 4h mark: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-capi-operator/267/pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-serial/1920845242664226816

I am happy to override it

/override ci/prow/e2e-aws-ovn-serial

[openshift-ci]

@damdo: Overrode contexts on behalf of damdo: ci/prow/e2e-aws-ovn-serial

Details

In response to this:

The serial job passed but timed out on tear-down hitting the 4h mark: https://prow.ci.openshift.org/view/gs/test-platform-results/pr-logs/pull/openshift_cluster-capi-operator/267/pull-ci-openshift-cluster-capi-operator-main-e2e-aws-ovn-serial/1920845242664226816

I am happy to override it

/override ci/prow/e2e-aws-ovn-serial

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-sigs/prow repository.

[damdo]

/test unit

[damdo]

/test unit

[openshift-ci]

@honza: all tests passed!

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

[damdo]

/cherry-pick release-4.19

[openshift-cherrypick-robot]

@damdo: new pull request created: #296

Details

In response to this:

/cherry-pick release-4.19

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-sigs/prow repository.

[damdo]

/retitle OCPBUGS-56104,OCPCLOUD-2893: Add related objects to must-gather config

[openshift-ci-robot]

@honza: Jira Issue OCPBUGS-56104: All pull requests linked via external trackers have merged:

Jira Issue OCPBUGS-56104 has been moved to the MODIFIED state.

Details

In response to this:

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-bot]

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-capi-operator
This PR has been included in build ose-cluster-capi-operator-container-v4.20.0-202505140744.p0.g39748f2.assembly.stream.el9.
All builds following this will include this PR.