Update godoc of CapacityReservationGroupID of AzureMachineSpec

[arkadeepsen]

What type of PR is this?
/kind cleanup

What this PR does / why we need it:
This PR updates the godoc of CapacityReservationGroupID of AzureMachineSpec. The updated godoc contains the explanation of what the requirements are for the CapacityReservationGroupID.

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 #4829

Special notes for your reviewer:

• cherry-pick candidate

TODOs:

• squashed commits
• includes documentation
• adds unit tests

Release note:

NONE

[k8s-ci-robot]

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

[mboersma]

Thanks @arkadeepsen!

The azuremachine_types.go file is used in generating the CRDs. So you need to run make generate and commit those changes or this will fail the make verify-gen check.

[mboersma]

/ok-to-test

[arkadeepsen]

Thanks @arkadeepsen!

The azuremachine_types.go file is used in generating the CRDs. So you need to run make generate and commit those changes or this will fail the make verify-gen check.

@mboersma my bad. Thanks for pointing that out. I have run make generate and pushed the changes.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.02%. Comparing base (43e72a9) to head (de135dc).
Report is 20 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4830      +/-   ##
==========================================
+ Coverage   62.01%   62.02%   +0.01%     
==========================================
  Files         201      201              
  Lines       16858    16858              
==========================================
+ Hits        10455    10457       +2     
+ Misses       5620     5618       -2     
  Partials      783      783              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[willie-yao]

I see the validation is already being done in ValidateCapacityReservationGroupID. I think the clarity helps but other fields that have similar requirements do not include this information in the godoc. @jackfrancis @mboersma what's your take on this? Was just wondering for the sake of consistency, otherwise lgtm.

[arkadeepsen]

I see the validation is already being done in ValidateCapacityReservationGroupID. I think the clarity helps but other fields that have similar requirements do not include this information in the godoc. @jackfrancis @mboersma what's your take on this? Was just wondering for the sake of consistency, otherwise lgtm.

Providing a context as to why the change in the godoc is introduced. In OpenShift specific APIs, we try to provide information in the godoc which will be beneficial for the user to create/update a CR. Though, as pointed out, the information is provided when the validation fails, it's good to have the information available beforehand.
P.S. I made the change to the godoc specific to CapacityReservationGroupID as I had worked on the PR which adds the field.

[mboersma]

@willie-yao it's a good point–we could argue it should be documented similarly for many other fields. I just assumed this change in particular would be (would have been) helpful for the requestor and perhaps others, and wasn't bothered by the inconsistency. But we should hear from other maintainers.

[willie-yao]

In OpenShift specific APIs, we try to provide information in the godoc which will be beneficial for the user to create/update a CR

Sounds good, it definitely doesn't hurt. I'll wait for another opinion from the team before lgtm but I'm good with this change!

[JoelSpeed]

/lgtm

Plus 1 for additional context to help users coming up with the correct input

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: f1440a9f88f5752096ad2940d2f4763c8c17d6ac

[mboersma]

/assign @willie-yao for approval

[k8s-ci-robot]

@mboersma: GitHub didn't allow me to assign the following users: for, approval.

Note that only kubernetes-sigs members with read permissions, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @willie-yao for approval

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.

[willie-yao]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: willie-yao

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 [willie-yao]

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