Automate generation of CiliumNode, CiliumIdentity, & CiliumEndpoint CRDs using controller-gen#11476
Merged
Automate generation of CiliumNode, CiliumIdentity, & CiliumEndpoint CRDs using controller-gen#11476
Conversation
This comment has been minimized.
This comment has been minimized.
2 tasks
bfb3150 to
6b97305
Compare
Member
|
Weirdly enough, even after specifying to generate short names, it still doesn't work. No mention of this not working on the upstream issue tracker. |
Member
Author
|
@christarazi this does the trick: diff --git a/pkg/k8s/apis/cilium.io/v2/types.go b/pkg/k8s/apis/cilium.io/v2/types.go
index 2b1c08a56..3ec858a8d 100644
--- a/pkg/k8s/apis/cilium.io/v2/types.go
+++ b/pkg/k8s/apis/cilium.io/v2/types.go
@@ -335,6 +335,7 @@ type CiliumClusterwideNetworkPolicyList struct {
// CiliumEndpoint is a CRD that represents an endpoint managed by Cilium.
// +k8s:openapi-gen=false
+// +kubebuilder:resource:shortName={cep,ciliumep}
type CiliumEndpoint struct {
// +k8s:openapi-gen=false
metav1.TypeMeta `json:",inline"` |
6c670cc to
ea9d913
Compare
Member
|
test-me-please |
This comment has been minimized.
This comment has been minimized.
9a11199 to
cebb54f
Compare
Member
|
#11607 brings in automatically generated CNP and CCNP. When that PR is merged, then it'll be the basis of this PR. The rest of this PR would be to add CiliumEndpoint, CiliumIdentity, and CiliumNode. |
370bacb to
76f19f6
Compare
aanm
commented
Aug 14, 2020
Member
Author
aanm
left a comment
There was a problem hiding this comment.
I can't approve my own PR 😢
Overall looks good, only a small comment
sayboras
reviewed
Aug 14, 2020
sayboras
approved these changes
Aug 14, 2020
Member
sayboras
left a comment
There was a problem hiding this comment.
One small question only, not a blocker 👍
This commit also adds a missing endpoint state, "invalid" This CRD is generated using https://book.kubebuilder.io/reference/generating-crd.html. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This CRD is generated using https://book.kubebuilder.io/reference/generating-crd.html. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This commit adds resource name generation via markers. It also updates
the validation schema via markers which the old method of hardcoding had
a few fields missing / incorrect.
Here's a summary of the main differences in the validation schema:
1) Old schema had eniTypes.ENISpec.MaxAllocate, which doesn't exist. It
exists under ipamTypes.IPAMSpec.MaxAllocate.
2) Old schema was missing eniTypes.ENISpec.{InstanceID,InstanceType},
which is now included in this new generated schema.
3) Old schema was missing EncryptionSpec, which is now included in this
new generated schema.
4) Old schema has azureTypes.AzureSpec.InstanceID, but that was removed
and deprecated in favor of ipamTypes.IPAMSpec.InstanceID. See PR
which forgot to remove this from the old validation schema. [1]
This CRD is generated using
https://book.kubebuilder.io/reference/generating-crd.html.
[1]: #10569
Signed-off-by: Chris Tarazi <chris@isovalent.com>
Kubernetes code tends to have very long lines because of package / variable names, and can make certain lines hard to parse. This commit attempts to make the code easier to parse. No functional changes were made. Signed-off-by: Chris Tarazi <chris@isovalent.com>
Previously, we only wanted to update a CRD when the CRD has an out-of-date schema version (which is inside the label "io.cilium.k8s.crd.schema.version".) Some CRDs like CiliumNode (CN) had a validation schema, but did not have a schema version (label mentioned above). This commit will allow all CRDs like CN to be updated, because it removes the check for the existance of the label. Signed-off-by: Chris Tarazi <chris@isovalent.com>
This bump is after generating the following CRDs using the controller-gen tool: - CiliumNode - CiliumIdentity - CiliumEndpoint Signed-off-by: Chris Tarazi <chris@isovalent.com>
These marker comments have no effect. They are likely leftovers from when the tooling was not as mature. We are able to generate (`make generate-k8s-api`) without issue. Signed-off-by: Chris Tarazi <chris@isovalent.com>
Member
|
Thanks for the reviews. Incremental diff |
d881c1b to
23750bb
Compare
Member
|
test-me-please |
1 similar comment
Member
|
test-me-please |
Member
|
@aanm (Pinging you because it's your own PR 😉 ) |
aanm
commented
Aug 18, 2020
Member
Author
aanm
left a comment
There was a problem hiding this comment.
I can't approve my own PR but LGTM 👍
tklauser
approved these changes
Aug 18, 2020
Member
|
Majority code owners approval and CI passing, marking ready to merge. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See #11476 (comment) for what this PR is about.
Followups are tracked here: #12737
Updates: #11142
Related: #12516
Previous PR body