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 all context.TODO occurences + fix manifests generation problem (E2E) #70
Conversation
@@ -53,8 +53,8 @@ help: ## Display this help. | |||
|
|||
.PHONY: manifests | |||
manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. | |||
$(CONTROLLER_GEN) crd:generateEmbeddedObjectMeta=true rbac:roleName=manager-role crd:maxDescLen=0 webhook paths="./..." output:crd:artifacts:config=config/crd/bases |
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.
is kustomize make install
working after this change ?
can you confirm ?
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.
No this is what I wrote above regarding the kubectl apply
.
make manifests
runs kubectl apply
and it won't work here because of the annotation.
Error: metadata.annotations: Too long: must have at most 262144 bytes
We need to decide if we want to try and remove the description, or try to bypass the kubectl apply
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 can find the two solutions and then we'll decide
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.
we can add --server-side
for the make manifest
in order to support the kubectl apply
operation. Added it, to the relevant make commands and to the e2e tests. I think its better to have the descriptions in the CRD.
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.
Left a comment on Makefile changes. cc @avtarOPS
thanks @itamar-marom |
fixes: #50 #64
Removing all
context.TODO
code from the controller. This shouldn't be in our code.Also removes the
crd:maxDescLen=0
option from the controller-gen command in order to fix the CI.This might be problematic since I'm not sure if
kubectl apply
will work on that CRD. (this command adds a 'last-applied` annotation of the entire CRD which is bigger that the annotation's allowed length)