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

Use apiextension v1 #186

Merged
merged 1 commit into from
Jun 14, 2021
Merged

Use apiextension v1 #186

merged 1 commit into from
Jun 14, 2021

Conversation

leonnicolas
Copy link
Collaborator

  • upgrade from apiextension v1beta1 to v1
  • generate yaml manifest for crd intead of applying it at runtime
    • users will have to apply the manifest with kubectl
  • kg and kgctl log an error if the crd is not present
  • now validation should actually work

Signed-off-by: leonnicolas leonloechner@gmx.de

@leonnicolas
Copy link
Collaborator Author

leonnicolas commented Jun 14, 2021

I will update the e2e tests in a follow up. Mainly because I would like to write something like

kgctl apply -f https://raw.git....crds.yaml

but the yaml manifest is not present in the main branch yet.

EDIT: nvm, kg crashes without the crd

Makefile Show resolved Hide resolved
Makefile Outdated
@@ -75,7 +76,13 @@ all-container-latest: $(addprefix container-latest-, $(ALL_ARCH))

all-push-latest: $(addprefix push-latest-, $(ALL_ARCH))

generate: client deepcopy informer lister openapi
generate: client deepcopy informer lister openapi crd
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
generate: client deepcopy informer lister openapi crd
generate: client deepcopy informer lister crd

Makefile Outdated
@@ -120,6 +127,17 @@ pkg/k8s/informers/kilo/v1alpha1/peer.go: .header pkg/k8s/apis/kilo/v1alpha1/type
rm -r github.com || true
go fmt ./pkg/k8s/informers/...

openapi: pkg/k8s/apis/kilo/v1alpha1/openapi_generated.go
Copy link
Owner

Choose a reason for hiding this comment

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

can we remove this stuff now?

Makefile Outdated
@@ -1,5 +1,5 @@
export GO111MODULE=on
.PHONY: push container clean container-name container-latest push-latest fmt lint test unit vendor header generate client deepcopy informer lister openapi manifest manfest-latest manifest-annotate manifest manfest-latest manifest-annotate release gen-docs e2e
.PHONY: push container clean container-name container-latest push-latest fmt lint test unit vendor header generate openapi crd client deepcopy informer lister manifest manfest-latest manifest-annotate manifest manfest-latest manifest-annotate release gen-docs e2e
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
.PHONY: push container clean container-name container-latest push-latest fmt lint test unit vendor header generate openapi crd client deepcopy informer lister manifest manfest-latest manifest-annotate manifest manfest-latest manifest-annotate release gen-docs e2e
.PHONY: push container clean container-name container-latest push-latest fmt lint test unit vendor header generate crd client deepcopy informer lister manifest manfest-latest manifest-annotate manifest manfest-latest manifest-annotate release gen-docs e2e

Copy link
Owner

@squat squat left a comment

Choose a reason for hiding this comment

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

lgtm man :) we can remove the openapi code right? then let's merge

tools.go Outdated
@@ -24,4 +24,5 @@ import (
_ "k8s.io/code-generator/cmd/informer-gen"
_ "k8s.io/code-generator/cmd/lister-gen"
_ "k8s.io/kube-openapi/cmd/openapi-gen"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
_ "k8s.io/kube-openapi/cmd/openapi-gen"

Copy link
Owner

@squat squat Jun 14, 2021

Choose a reason for hiding this comment

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

I think this is the last thing, then make vendor

 - upgrade from apiextension v1beta1 to v1
 - generate yaml manifest for crd intead of applying it at runtime
  - users will have to apply the manifest with kubectl
 - kg and kgctl log an error if the crd is not present
 - now validation should actually work

Signed-off-by: leonnicolas <leonloechner@gmx.de>
Copy link
Owner

@squat squat left a comment

Choose a reason for hiding this comment

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

nice :))))))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

apiextensions.k8s.io/v1beta1 CustomResourceDefinition is deprecated in v1.16+, unavailable in v1.22+
2 participants