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

fix: verify generation in record status #706

Merged
merged 6 commits into from
Oct 26, 2021

Conversation

gxthrj
Copy link
Contributor

@gxthrj gxthrj commented Oct 11, 2021

Please answer these questions before submitting a pull request

Use the generation field under the resource and ObservedGeneration under the status to compare to prevent the status from being recorded repeatedly.

ref: https://github.com/kubernetes/community/blob/master/contributors/design-proposals/api-machinery/customresources-subresources.md#status-behavior

@codecov-commenter
Copy link

codecov-commenter commented Oct 11, 2021

Codecov Report

Merging #706 (a02c42e) into master (a01888b) will increase coverage by 67.63%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           master      #706       +/-   ##
============================================
+ Coverage   32.36%   100.00%   +67.63%     
============================================
  Files          65         1       -64     
  Lines        6488         1     -6487     
============================================
- Hits         2100         1     -2099     
+ Misses       4137         0     -4137     
+ Partials      251         0      -251     
Impacted Files Coverage Δ
pkg/ingress/apisix_tls.go
pkg/ingress/status.go
pkg/ingress/apisix_cluster_config.go
pkg/ingress/apisix_consumer.go
pkg/ingress/secret.go
pkg/ingress/apisix_upstream.go
pkg/ingress/apisix_route.go
pkg/apisix/cluster.go
pkg/apisix/cache/memdb.go
pkg/apisix/schema.go
... and 54 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 a01888b...a02c42e. Read the comment docs.

@tao12345666333 tao12345666333 added this to In progress in v1.4 Planning via automation Oct 13, 2021
@gxthrj gxthrj added this to the 1.4.0 milestone Oct 21, 2021
@lingsamuel
Copy link
Member

Can we add tests?

@tokers
Copy link
Contributor

tokers commented Oct 22, 2021

@gxthrj Could you add more descriptions on this PR so that we can know the context of it, frankly, I don't know the exact purpose of Generation.

@tao12345666333 tao12345666333 added the bug Something isn't working label Oct 22, 2021
@tao12345666333
Copy link
Member

Can we add tests?

+1 @gxthrj PTAL

@tao12345666333
Copy link
Member

@gxthrj please check the CI error

@gxthrj
Copy link
Contributor Author

gxthrj commented Oct 26, 2021

@gxthrj please check the CI error

The CI has passsed.

Copy link
Member

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

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

CI passed

v1.4 Planning automation moved this from In progress to Reviewer approved Oct 26, 2021
@tao12345666333 tao12345666333 merged commit 2a73216 into apache:master Oct 26, 2021
v1.4 Planning automation moved this from Reviewer approved to Done Oct 26, 2021
@tao12345666333
Copy link
Member

Should we close #671 and #682?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

bug: status subresource can not be record when using CRD apiextensions.k8s.io/v1
5 participants