Skip to content

Conversation

@weapons97
Copy link
Contributor

Realted to: #3878
Add admission validate for hypernode

@volcano-sh-bot volcano-sh-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Dec 16, 2024
@weapons97 weapons97 force-pushed the network-topo-admission-validate-hypernode branch 3 times, most recently from c54f1b8 to 8f8a935 Compare December 16, 2024 10:44
}

// validateHyperNodeName is to validate hypernode name.
func validateHyperNodeName(value string, fldPath *field.Path) field.ErrorList {
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to add this func, kube-apiserver already has validation func to validate the crd resource name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@weapons97 weapons97 force-pushed the network-topo-admission-validate-hypernode branch 3 times, most recently from b54065a to cccc032 Compare December 18, 2024 09:00
@volcano-sh-bot volcano-sh-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 9, 2025
@weapons97 weapons97 force-pushed the network-topo-admission-validate-hypernode branch from cccc032 to decacbf Compare January 14, 2025 08:56
@volcano-sh-bot volcano-sh-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 14, 2025
@weapons97 weapons97 force-pushed the network-topo-admission-validate-hypernode branch from decacbf to 8a09bd6 Compare January 14, 2025 09:00
@JesseStutler
Copy link
Member

Great!

@JesseStutler
Copy link
Member

cc @hwdef @lowang-bh @Monokaix

}

// validateHyperNode is to validate hypernode.
func validateHyperNode(hypernode *hypernodev1alpha1.HyperNode) error {
Copy link
Member

Choose a reason for hiding this comment

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

Does hypernode will be nil?

Copy link
Member

Choose a reason for hiding this comment

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

Where is this function used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it used by line 76 in AdmitHyperNode functions, and hypernode must not nil because schema.DecodeHyperNode will return error when res has error

errs := field.ErrorList{}
resourcePath := field.NewPath("")

for _, member := range hypernode.Spec.Members {
Copy link
Member

Choose a reason for hiding this comment

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

Will len(hypernode.Spec.Members) will be 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, i changed

@weapons97 weapons97 force-pushed the network-topo-admission-validate-hypernode branch from 86aad15 to 51bbe9d Compare January 16, 2025 10:29
@Monokaix
Copy link
Member

/ok-to-test
/approve

@volcano-sh-bot volcano-sh-bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Jan 16, 2025
@volcano-sh-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Monokaix

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@volcano-sh-bot volcano-sh-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 16, 2025
@weapons97 weapons97 force-pushed the network-topo-admission-validate-hypernode branch from 51bbe9d to 7c25611 Compare January 16, 2025 12:25
Signed-off-by: weapons97 <[email protected]>
@weapons97 weapons97 force-pushed the network-topo-admission-validate-hypernode branch from 7c25611 to 296c650 Compare January 16, 2025 13:23
Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

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

/lgtm

@volcano-sh-bot volcano-sh-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2025
@volcano-sh-bot volcano-sh-bot merged commit d363c00 into volcano-sh:network-topology Jan 16, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants