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

Add validations for SubnetPort CRD #869

Merged
merged 1 commit into from
Nov 22, 2024

Conversation

lxiaopei
Copy link
Contributor

@lxiaopei lxiaopei commented Nov 5, 2024

Add validations for SubnetPort CR to specify one of subnet or subnetSet under spec.

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.38%. Comparing base (5d7eef7) to head (fb2defe).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #869      +/-   ##
==========================================
+ Coverage   73.36%   73.38%   +0.02%     
==========================================
  Files         109      109              
  Lines       15235    15248      +13     
==========================================
+ Hits        11177    11190      +13     
  Misses       3307     3307              
  Partials      751      751              
Flag Coverage Δ
unit-tests 73.38% <ø> (+0.02%) ⬆️
Files with missing lines Coverage Δ
pkg/apis/vpc/v1alpha1/subnetport_types.go 100.00% <ø> (ø)
...kg/controllers/subnetport/subnetport_controller.go 75.68% <ø> (-0.21%) ⬇️

... and 2 files with indirect coverage changes

---- 🚨 Try these New Features:

@lxiaopei
Copy link
Contributor Author

lxiaopei commented Nov 6, 2024

/e2e

@lxiaopei lxiaopei force-pushed the topic/lxiaopei/subnetport_spec branch from f2764e0 to d1d99df Compare November 6, 2024 08:20
@lxiaopei
Copy link
Contributor Author

lxiaopei commented Nov 6, 2024

e2e

@lxiaopei
Copy link
Contributor Author

lxiaopei commented Nov 6, 2024

/e2e

@yanjunz97
Copy link
Contributor

Shall we remove the check in subnetport controller for this validation?

if len(subnetPort.Spec.SubnetSet) > 0 && len(subnetPort.Spec.Subnet) > 0 {
err := errors.New("subnet and subnetset should not be configured at the same time")
log.Error(err, "failed to get subnet/subnetset of the subnetport", "subnetport", req.NamespacedName)
updateFail(r, ctx, subnetPort, &err)
return common.ResultNormal, err
}

@lxiaopei lxiaopei force-pushed the topic/lxiaopei/subnetport_spec branch from d1d99df to e04dc93 Compare November 7, 2024 03:00
@lxiaopei
Copy link
Contributor Author

lxiaopei commented Nov 7, 2024

Shall we remove the check in subnetport controller for this validation?

if len(subnetPort.Spec.SubnetSet) > 0 && len(subnetPort.Spec.Subnet) > 0 {
err := errors.New("subnet and subnetset should not be configured at the same time")
log.Error(err, "failed to get subnet/subnetset of the subnetport", "subnetport", req.NamespacedName)
updateFail(r, ctx, subnetPort, &err)
return common.ResultNormal, err
}

thanks,removed.

dantingl
dantingl previously approved these changes Nov 7, 2024
@dantingl dantingl requested a review from yanjunz97 November 7, 2024 06:45
@yanjunz97
Copy link
Contributor

Fix the UT?

@lxiaopei lxiaopei force-pushed the topic/lxiaopei/subnetport_spec branch 4 times, most recently from 71c3220 to 845e018 Compare November 8, 2024 07:12
yanjunz97
yanjunz97 previously approved these changes Nov 8, 2024
@lxiaopei lxiaopei force-pushed the topic/lxiaopei/subnetport_spec branch 7 times, most recently from 808b2b3 to a707ed5 Compare November 21, 2024 10:13
@lxiaopei
Copy link
Contributor Author

/e2e

yanjunz97
yanjunz97 previously approved these changes Nov 22, 2024
Copy link
Contributor

@yanjunz97 yanjunz97 left a comment

Choose a reason for hiding this comment

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

LGTM

@lxiaopei lxiaopei force-pushed the topic/lxiaopei/subnetport_spec branch 2 times, most recently from 1a351aa to a336862 Compare November 22, 2024 06:37
Add validations for SubnetPort CR to specify one of subnet or subnetSet under spec.
@lxiaopei lxiaopei force-pushed the topic/lxiaopei/subnetport_spec branch from a336862 to fb2defe Compare November 22, 2024 07:20
@lxiaopei lxiaopei merged commit 1cf8cb7 into vmware-tanzu:main Nov 22, 2024
2 checks passed
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.

5 participants