Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Export GroupVersionResource and GroupKind variables #212

Merged
merged 1 commit into from
Oct 1, 2021

Conversation

RedbackThomson
Copy link
Contributor

Fixes aws-controllers-k8s/community#998

Description of changes:
GroupVersionResource is used by dynamic clients to specify the CRD to be CRUD'd.
GroupKind is sometimes used in place where the version is not necessarily specified. I chose to export this mainly because it could be used down the line by other dynamic clients.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Member

@a-hilaly a-hilaly left a comment

Choose a reason for hiding this comment

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

👍
/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2021
@a-hilaly
Copy link
Member

a-hilaly commented Oct 1, 2021

/lgtm cancel

@ack-bot ack-bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2021
@@ -32,7 +33,7 @@ type resourceDescriptor struct {
// GroupKind returns a Kubernetes metav1.GroupKind struct that describes the
// API Group and Kind of CRs described by the descriptor
func (d *resourceDescriptor) GroupKind() *metav1.GroupKind {
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this method?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, because the runtime doesn't import the resource package itself (it loads the resource manager factory from a registry) but still needs to know the GroupKind:

https://github.com/aws-controllers-k8s/runtime/blob/8e4dc5ecc41f9370e413c3be4ee88baf67ecbcd1/pkg/runtime/reconciler.go#L65-L72

Copy link
Collaborator

@jaypipes jaypipes left a comment

Choose a reason for hiding this comment

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

👍

@@ -32,7 +33,7 @@ type resourceDescriptor struct {
// GroupKind returns a Kubernetes metav1.GroupKind struct that describes the
// API Group and Kind of CRs described by the descriptor
func (d *resourceDescriptor) GroupKind() *metav1.GroupKind {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, because the runtime doesn't import the resource package itself (it loads the resource manager factory from a registry) but still needs to know the GroupKind:

https://github.com/aws-controllers-k8s/runtime/blob/8e4dc5ecc41f9370e413c3be4ee88baf67ecbcd1/pkg/runtime/reconciler.go#L65-L72

@jaypipes
Copy link
Collaborator

jaypipes commented Oct 1, 2021

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 1, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Oct 1, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jaypipes, RedbackThomson, vijtrip2

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 [RedbackThomson,jaypipes,vijtrip2]

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

@ack-bot ack-bot merged commit b6b4012 into aws-controllers-k8s:main Oct 1, 2021
@RedbackThomson RedbackThomson deleted the gvr branch October 1, 2021 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatically generate a dynamic client or a fargateprofiles client with a runtime scheme
5 participants