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

ByoClusterTemplate added for cluster class support #540

Merged
merged 1 commit into from
May 11, 2022

Conversation

shivi28
Copy link
Contributor

@shivi28 shivi28 commented May 10, 2022

What this PR does / why we need it:

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

Additional information

Special notes for your reviewer

@vmwclabot
Copy link

@shivi28, you must sign our contributor license agreement before your changes are merged. Click here to sign the agreement. If you are a VMware employee, read this for further instruction.

@vmwclabot
Copy link

@shivi28, your company's legal contact has approved your signed contributor license agreement. It will also be reviewed by VMware, but the merge can proceed.

@shivi28
Copy link
Contributor Author

shivi28 commented May 10, 2022

Requesting @dharmjit to kindly review this

@codecov-commenter
Copy link

Codecov Report

Merging #540 (4647e57) into main (a87f08c) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #540      +/-   ##
==========================================
+ Coverage   63.63%   63.67%   +0.03%     
==========================================
  Files          25       26       +1     
  Lines        1936     1938       +2     
==========================================
+ Hits         1232     1234       +2     
  Misses        625      625              
  Partials       79       79              
Impacted Files Coverage Δ
apis/infrastructure/v1beta1/byoclustertemplate.go 100.00% <100.00%> (ø)

Copy link
Contributor

@sachinkumarsingh092 sachinkumarsingh092 left a comment

Choose a reason for hiding this comment

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

Is there no need for any observed state for ByoClusterTemplate i.e a ByoClusterTemplateStatus for cluster class?

@shivi28
Copy link
Contributor Author

shivi28 commented May 10, 2022

Is there no need for any observed state for ByoClusterTemplate i.e a ByoClusterTemplateStatus for cluster class?

Right now there is no benefit of having status subresource, but based on use case we can extend it in future.

Copy link
Contributor

@dharmjit dharmjit left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@sachinkumarsingh092 sachinkumarsingh092 left a comment

Choose a reason for hiding this comment

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

LGTM

@sachinkumarsingh092 sachinkumarsingh092 merged commit ca6f3e9 into vmware-tanzu:main May 11, 2022
@vmwclabot
Copy link

@shivi28, VMware has rejected your signed contributor license agreement. The change has already been merged, but it will be backed out by the project maintainers if the agreement is not resigned in a timely manner. Click here to resign the agreement.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add clusterclass template
5 participants