Skip to content

Conversation

@Arvindthiru
Copy link
Contributor

@Arvindthiru Arvindthiru commented May 28, 2025

Description of your changes

Fixes #

I have:

  • Run make reviewable to ensure this PR is ready for review.

How has this code been tested

Special notes for your reviewer

@Arvindthiru Arvindthiru marked this pull request as ready for review June 11, 2025 01:37
}

// Type assertion to make sure we have a CRD.
crd, ok := obj.(*apiextensionsv1.CustomResourceDefinition)
Copy link
Contributor

Choose a reason for hiding this comment

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

is the GVK returned not enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to convert obj -> crd to access crd.Name to get object existing on the cluster. We can use check to achieve both (type assertion and get object name)

Copy link
Contributor

Choose a reason for hiding this comment

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

what's that GVK? is that not always CustomResourceDefinition? or that's the real GVK of that CRD (i.e. crp.kube-fleet.io)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image It's CustomResourceDefinition

@Arvindthiru Arvindthiru changed the title feat: add sidecar init container to install fleet CRDs feat: add init container to install fleet CRDs Jun 13, 2025
}

// Type assertion to make sure we have a CRD.
crd, ok := obj.(*apiextensionsv1.CustomResourceDefinition)
Copy link
Contributor

Choose a reason for hiding this comment

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

what's that GVK? is that not always CustomResourceDefinition? or that's the real GVK of that CRD (i.e. crp.kube-fleet.io)?

Arvind Thirumurugan added 2 commits June 25, 2025 10:26
Copy link
Contributor

@ryanzhang-oss ryanzhang-oss left a comment

Choose a reason for hiding this comment

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

we need to add metrics and alert in the next PR

@Arvindthiru Arvindthiru merged commit aa1e6f8 into Azure:main Jun 25, 2025
19 of 20 checks passed
Arvindthiru added a commit to Arvindthiru/fleet-public that referenced this pull request Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants