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

.*: implement the DNS chaos CRD API #935

Merged
merged 15 commits into from
Oct 13, 2020

Conversation

WangXiangUSTC
Copy link
Contributor

@WangXiangUSTC WangXiangUSTC commented Sep 21, 2020

What problem does this PR solve?

Add a new CRD DNS chaos, and implement this CRD's API.

This pr is part of #860, #860 is too big, so I will split it to multiple small prs.

Checklist

Tests

  • Unit test
  • E2E test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Breaking backward compatibility

Related changes

  • Need to update the documentation

Does this PR introduce a user-facing change?

NONE

Signed-off-by: xiang <xiang13225080@163.com>
Signed-off-by: xiang <xiang13225080@163.com>
Signed-off-by: xiang <xiang13225080@163.com>
Signed-off-by: xiang <xiang13225080@163.com>
@codecov-commenter
Copy link

Codecov Report

Merging #935 into master will decrease coverage by 7.16%.
The diff coverage is 28.08%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #935      +/-   ##
==========================================
- Coverage   55.78%   48.61%   -7.17%     
==========================================
  Files          68       76       +8     
  Lines        4383     4371      -12     
==========================================
- Hits         2445     2125     -320     
- Misses       1768     2042     +274     
- Partials      170      204      +34     
Impacted Files Coverage Δ
api/v1alpha1/common_validation.go 100.00% <ø> (ø)
api/v1alpha1/common_webhook.go 100.00% <ø> (ø)
api/v1alpha1/dnschaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/iochaos_types.go 37.03% <0.00%> (-2.97%) ⬇️
api/v1alpha1/kernelchaos_types.go 18.51% <0.00%> (-1.49%) ⬇️
api/v1alpha1/kernelchaos_webhook.go 84.21% <ø> (-0.98%) ⬇️
api/v1alpha1/kinds.go 27.27% <ø> (+0.60%) ⬆️
api/v1alpha1/networkchaos_types.go 19.54% <0.00%> (-3.44%) ⬇️
api/v1alpha1/podchaos_types.go 33.33% <0.00%> (-3.81%) ⬇️
api/v1alpha1/podchaos_webhook.go 80.76% <ø> (-1.20%) ⬇️
... and 123 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af2d2a1...23812f1. Read the comment docs.

@WangXiangUSTC
Copy link
Contributor Author

/run-all-tests

Signed-off-by: xiang <xiang13225080@163.com>
Signed-off-by: xiang <xiang13225080@163.com>
Copy link
Member

@Gallardot Gallardot left a comment

Choose a reason for hiding this comment

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

Rest LGTM

return ValidatePodMode(in.Spec.Value, in.Spec.Mode, spec.Child("value"))
}

func (in *DNSChaosSpec) validateAction(action *field.Path) field.ErrorList {
Copy link
Member

@Gallardot Gallardot Sep 29, 2020

Choose a reason for hiding this comment

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

I found a similar logic above using kubebuilder:validation.

// +kubebuilder:validation:Enum=error;random
Action DNSChaosAction `json:"action"`

kubebuilder:validation is enough.
I don't think this method is necessary at present.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I removed these two functions

return allErrs
}

func (in *DNSChaosSpec) validateScope(scope *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.

ditto

// +kubebuilder:validation:Enum=outer;inner;all
Scope DNSChaosScope `json:"scope"`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

Signed-off-by: xiang <xiang13225080@163.com>
Signed-off-by: xiang <xiang13225080@163.com>
fewdan
fewdan previously approved these changes Oct 9, 2020
Copy link
Member

@fewdan fewdan left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

codecov-io commented Oct 9, 2020

Codecov Report

Merging #935 into master will decrease coverage by 8.38%.
The diff coverage is 26.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #935      +/-   ##
==========================================
- Coverage   55.78%   47.40%   -8.39%     
==========================================
  Files          68       72       +4     
  Lines        4383     4367      -16     
==========================================
- Hits         2445     2070     -375     
- Misses       1768     2087     +319     
- Partials      170      210      +40     
Impacted Files Coverage Δ
api/v1alpha1/common_types.go 0.00% <0.00%> (ø)
api/v1alpha1/common_validation.go 100.00% <ø> (ø)
api/v1alpha1/common_webhook.go 100.00% <ø> (ø)
api/v1alpha1/dnschaos_type.go 0.00% <0.00%> (ø)
api/v1alpha1/dnschaos_webhook.go 0.00% <0.00%> (ø)
api/v1alpha1/httpchaos_types.go 0.00% <0.00%> (ø)
api/v1alpha1/iochaos_types.go 0.00% <ø> (-40.00%) ⬇️
api/v1alpha1/kernelchaos_types.go 0.00% <ø> (-20.00%) ⬇️
api/v1alpha1/kernelchaos_webhook.go 100.00% <ø> (+14.81%) ⬆️
api/v1alpha1/kinds.go 27.27% <ø> (+0.60%) ⬆️
... and 113 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf9e1ca...4e42158. Read the comment docs.

@WangXiangUSTC
Copy link
Contributor Author

@Gallardot PTAL again

YangKeao
YangKeao previously approved these changes Oct 10, 2020
Copy link
Member

@YangKeao YangKeao left a comment

Choose a reason for hiding this comment

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

LGTM

ti-srebot
ti-srebot previously approved these changes Oct 10, 2020
@cwen0
Copy link
Member

cwen0 commented Oct 10, 2020

@WangXiangUSTC Do we need to update helm files and install.sh?

Signed-off-by: xiang <xiang13225080@163.com>
@WangXiangUSTC
Copy link
Contributor Author

@WangXiangUSTC Do we need to update helm files and install.sh?

OK, seems that I forget to update them

Signed-off-by: xiang <xiang13225080@163.com>
Signed-off-by: xiang <xiang13225080@163.com>
@WangXiangUSTC
Copy link
Contributor Author

@cwen0 this pr is refactored by chaos-builder, PTAL again

Copy link
Member

@cwen0 cwen0 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
Member

@Gallardot Gallardot left a comment

Choose a reason for hiding this comment

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

LGTM

@WangXiangUSTC
Copy link
Contributor Author

/merge

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 8505f11 into chaos-mesh:master Oct 13, 2020
@WangXiangUSTC WangXiangUSTC deleted the xiang/dns_chaos_api branch October 13, 2020 05:48
sjwsl pushed a commit to sjwsl/chaos-mesh that referenced this pull request May 6, 2021
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.

None yet

8 participants