-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
📖 (doc): getting started: fix information about make command for generating manifests #4306
📖 (doc): getting started: fix information about make command for generating manifests #4306
Conversation
Welcome @vtrenton! |
Hi @vtrenton. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Also fixed a simple yam -> yaml in the same section. |
docs/book/src/getting-started.md
Outdated
@@ -125,10 +125,10 @@ Now, see our example fully completed. | |||
|
|||
#### Generating manifests with the specs and validations | |||
|
|||
To generate the required CRDs we will run `make generate` command, which will call [controller-gen][controller-gen] | |||
To generate the required CRDs we will run `make manifests` command, which will call [controller-gen][controller-gen] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we need the macke generate right?
Since we changed the spec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick response!
make generate doesn't seem to do what this is saying this will do... Sorry very new to kubebuilder so I could be doing something wrong here:
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ tree config/crd/
config/crd/
├── kustomization.yaml
└── kustomizeconfig.yaml
1 directory, 2 files
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ make generate
/home/trent/Source/kubebuilder-demo/memcached-operator/bin/controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ tree config/crd/
config/crd/
├── kustomization.yaml
└── kustomizeconfig.yaml
1 directory, 2 files
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ make manifests
/home/trent/Source/kubebuilder-demo/memcached-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ tree config/crd/
config/crd/
├── bases
│ └── cache.example.com_memcacheds.yaml
├── kustomization.yaml
└── kustomizeconfig.yaml
2 directories, 3 files
just following along with the docs and running into this issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh I see what you're saying - we need both commands I expect. that's a great point! let me see if I can come up with something more clean than make generate && make manifests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes both.
maybe:
To generate the required CRDs we will run
make generate
command to generate ... andmake manisftes
to generate the manifests under .... , which will call [controller-gen][controller-gen]
Something like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess make build
would be the "one-shot" do everything approach but it looks like I don't need to run make generate
before running make manifests
if my goal is generating manifest.
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ ls config/crd/
kustomization.yaml kustomizeconfig.yaml
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ ls api/v1alpha1/
groupversion_info.go memcached_types.go
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ make manifests
/home/trent/Source/kubebuilder-demo/memcached-operator/bin/controller-gen rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $ ls -al config/crd/bases/
total 16
drwxr-xr-x 2 trent users 4096 Nov 8 12:37 .
drwxr-xr-x 3 trent users 4096 Nov 8 12:37 ..
-rw-r--r-- 1 trent users 4921 Nov 8 12:37 cache.example.com_memcacheds.yaml
(.venv) (devbox) [trent@Krum:~/Source/kubebuilder-demo/memcached-operator] [master] $
probably still need to run make generate
at some point to generate the deep copy stuff...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To generate all required files, run make generate
to create the DeepCopy implementations in api/v1alpha1/zz_generated.deepcopy.go
, followed by make manifests
to generate the CRD manifests under config/crd/bases
. Both commands use [controller-gen][controller-gen] with different flags for code and manifest generation respectively.
does that wording sound OK? just want to make sure i'm not missing anything critical before making another commit - updating. Thanks for your help! I haven't read too far ahead in the docs yet so i'm not sure how verbose vs concise the wording should be!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
I loved. Just a tiny refinement :-)
To generate all required files:
1. Run `make generate` to create the DeepCopy implementations in `api/v1alpha1/zz_generated.deepcopy.go`.
2. Then, run `make manifests` to generate the CRD manifests under `config/crd/bases` and a sample for it under `config/crd/samples`.
Both commands use [controller-gen][controller-gen] with different flags for code and manifest generation, respectively.
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohhh that is a wonderful callout in reguards to the sample dir as well!
That is perfect - thanks a bunch for the help, I've commited the changes!
to generate the CRD manifest, which is located under the `config/crd/bases` directory. | ||
|
||
<details><summary><code>config/crd/bases/cache.example.com_memcacheds.yam</code>: Our Memcached CRD</summary> | ||
<details><summary><code>config/crd/bases/cache.example.com_memcacheds.yaml</code>: Our Memcached CRD</summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Hi @vtrenton It is all fine now. Really thank you for the contribution 🥇 |
98ac79d
to
c372328
Compare
Thanks @camilamacedo86 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 🥇
/approved
/lgtm
/ok-to-test
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, vtrenton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
make generate does not generate the CRDs in config/crd/base the correct command to accomplish this is make manifests. See Makefile:
fixes: #4305