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 toEntities/fromEntities CRD description missing options #22279

Merged
merged 4 commits into from Dec 10, 2022

Conversation

slayer321
Copy link
Contributor

@slayer321 slayer321 commented Nov 20, 2022

Signed-off-by: Sachin Maurya sachin.maurya7666@gmail.com

add all the Entities options from docs

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

Fixes: #22235

@slayer321 slayer321 requested a review from a team as a code owner November 20, 2022 07:42
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 20, 2022
pchaigno
pchaigno previously approved these changes Nov 21, 2022
@pchaigno pchaigno dismissed their stale review November 21, 2022 11:29

Many CI tests are still failing, some changes are missing.

@pchaigno pchaigno marked this pull request as draft November 21, 2022 11:29
@pchaigno pchaigno added sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/misc This PR makes changes that have no direct user impact. labels Nov 21, 2022
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 21, 2022
@pchaigno
Copy link
Member

@slayer321 Thanks for the pull request! Please have a look at the failing CI checks. Some of them are failing because of missing changes (e.g., files that need to be regenerated).

I've switched the PR to draft mode until this is addressed.

@slayer321
Copy link
Contributor Author

@pchaigno When I'm running this command make generate-k8s-apiin cilium root dir I'm getting below error

Set $GOPATH to a directory containing the Cilium repository at $GOPATH/src/github.com/cilium/cilium.
The current working directory must be the repository root for code generation to work correctly.
make: *** [Makefile:329: generate-k8s-api] Error 1

This is where my cilium project resides /home/sachinmaurya/go/src/github.com/cilium
my GOPATH is /home/sachinmaurya/go ... as I can see that my cilium project is already in the dir it is mention .. I'm not getting why it is failing

@pchaigno
Copy link
Member

@slayer321 Could you ask in Slack? I'm not seeing the issue but others may spot it.

@chancez
Copy link
Contributor

chancez commented Nov 23, 2022

This is where my cilium project resides /home/sachinmaurya/go/src/github.com/cilium my GOPATH is /home/sachinmaurya/go ... as I can see that my cilium project is already in the dir it is mention .. I'm not getting why it is failing

It should be at /home/sachinmaurya/go/src/github.com/cilium/cilium, note the extra cilium directory.

@slayer321 slayer321 force-pushed the slayer321/crd-description-missing branch from 9b1095c to 1263445 Compare November 25, 2022 15:40
Signed-off-by: Sachin Maurya <sachin.maurya7666@gmail.com>
@slayer321 slayer321 force-pushed the slayer321/crd-description-missing branch from 1263445 to e439c2d Compare November 26, 2022 05:42
@slayer321
Copy link
Contributor Author

slayer321 commented Nov 26, 2022

@chancez I tried running the make generate-k8s-api command and it runs successfully without any errors and there was not changes in any of the files.. But now I can see that the generate-k8s-api CI is still failing and says I need to run the make commands ..
I don't understand what can be the reason for that 🤔

@aanm aanm added the kind/community-contribution This was a contribution made by a community member. label Dec 1, 2022
@aanm
Copy link
Member

aanm commented Dec 5, 2022

hey @slayer321, the changes in this PR are modifying a auto-generated file. The changes should be done here and then you should run the make generate-k8s-api. Let me know if this helps. Once you have submitted your changes don't forget to mark your PR as "Ready for review" Thank you.

@slayer321 slayer321 force-pushed the slayer321/crd-description-missing branch 2 times, most recently from e1051d7 to 0c6ae96 Compare December 5, 2022 16:44
@maintainer-s-little-helper

This comment was marked as resolved.

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 5, 2022
Signed-off-by: --global <sachin.maurya7666@gmail.com>
@slayer321 slayer321 force-pushed the slayer321/crd-description-missing branch from 0c6ae96 to 54bca27 Compare December 5, 2022 16:50
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Dec 5, 2022
Signed-off-by: slayer321 <sachin.maurya7666@gmail.com>
@slayer321
Copy link
Contributor Author

Hey @aanm , I made the changes in the file you mentioned and then when I run make generate-k8s-api command .. it runs successfully without any errors and It didn't modified the crds yaml file for me ..

Below I'm sharing the exit code for the make command

Generating deepcopy funcs
go run github.com/cilium/deepequal-gen --input-dirs "github.com/cilium/cilium/pkg/bpf,github.com/cilium/cilium/pkg/k8s,github.com/cilium/cilium/pkg/labels,github.com/cilium/cilium/pkg/loadbalancer,github.com/cilium/cilium/pkg/tuple,github.com/cilium/cilium/pkg/recorder" -O zz_generated.deepequal --go-header-file "/home/sachinmaurya/go/src/github.com/cilium/cilium/hack/custom-boilerplate.go.txt"


sachinmaurya@sachin:~/go/src/github.com/cilium/cilium (slayer321/crd-description-missing)$ echo $?
0

@aanm
Copy link
Member

aanm commented Dec 6, 2022

@slayer321 Also run make manifests

Signed-off-by: slayer321 <sachin.maurya7666@gmail.com>
@slayer321 slayer321 marked this pull request as ready for review December 6, 2022 13:35
@slayer321 slayer321 requested a review from a team as a code owner December 6, 2022 13:35
@aanm aanm added the area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. label Dec 10, 2022
@aanm aanm merged commit 7801098 into cilium:master Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. kind/community-contribution This was a contribution made by a community member. release-note/misc This PR makes changes that have no direct user impact. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

toEntities/fromEntities CRD description missing some options
4 participants