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 deepcopy generator checker #11165

Merged
merged 4 commits into from Apr 29, 2020
Merged

Add deepcopy generator checker #11165

merged 4 commits into from Apr 29, 2020

Conversation

aanm
Copy link
Member

@aanm aanm commented Apr 27, 2020

To avoid code from being merged without being regenerated, this commit adds a checker, that will only run in the CI, to verify if there is code ungenerated.

@aanm aanm added kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Apr 27, 2020
@aanm aanm requested review from a team as code owners April 27, 2020 09:45
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Apr 27, 2020
@aanm
Copy link
Member Author

aanm commented Apr 27, 2020

test-me-please

@aanm
Copy link
Member Author

aanm commented Apr 27, 2020

test-me-please

@coveralls
Copy link

coveralls commented Apr 27, 2020

Coverage Status

Coverage decreased (-0.01%) to 44.783% when pulling 98728bb on pr/add-generator-checker into eeb182b on master.

Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Looks good beside one potential typo

contrib/scripts/check-k8s-code-gen.sh Outdated Show resolved Hide resolved
k8s deepcopy generator can have multiple groups for the same version
passed as a parameter. To speed up the code generation we can merge all
of them together.

The amount of time decreased from 50s to 16s.

Signed-off-by: André Martins <andre@cilium.io>
gandro and others added 3 commits April 27, 2020 19:03
The parent `StatusResponse` struct requires it to perform a deep copy.

Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
Signed-off-by: André Martins <andre@cilium.io>
To avoid code from being merged without being regenerated, this commit
adds a checker, that will only run in the CI, to verify if there is code
ungenerated.

Signed-off-by: André Martins <andre@cilium.io>
@aanm
Copy link
Member Author

aanm commented Apr 27, 2020

test-me-please

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks good, the checker will be great.

Does any of the fix commits need a fixes: tag? I think the code missing deepcopy was added recently and no backport is necessary here, right? (Just double-checking)

@aanm
Copy link
Member Author

aanm commented Apr 28, 2020

@qmonnet that is correct. I didn't had a fixes because it because of multiple commits and this is always happening.

@qmonnet
Copy link
Member

qmonnet commented Apr 28, 2020

Multiple GKE failures, all looking like known flakes:

Test Result (3 failures / +1)

    Suite-k8s-1.14.K8sDatapathConfig MonitorAggregation Checks that monitor aggregation restricts notifications
    Suite-k8s-1.14.K8sServicesTest Checks service across nodes Tests NodePort (kube-proxy)
    Suite-k8s-1.14.K8sServicesTest Checks service across nodes with L7 policy Tests NodePort with L7 Policy

test-gke

@aanm
Copy link
Member Author

aanm commented Apr 28, 2020

@qmonnet GKE is not marked as "Required" for a PR to be merged.

@aanm
Copy link
Member Author

aanm commented Apr 28, 2020

Cilium-Tests-With-Kernel was renamed to K8s-1.17-Kernel-4.19

@qmonnet qmonnet merged commit c8db262 into master Apr 29, 2020
1.8.0 automation moved this from In progress to Merged Apr 29, 2020
@qmonnet qmonnet deleted the pr/add-generator-checker branch April 29, 2020 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants