bonny icon indicating copy to clipboard operation
bonny copied to clipboard

The current CRD from `mix bonny.gen.manifest` doesn't work on later version of k8s

Open FreedomBen opened this issue 2 years ago • 4 comments

The CRD v1beta1 has been removed from later versions of k8s (v1.20+). Format has changed from v1beta1 to v1 and it is incompatible, so some non-trivial changes are required.

For reference/convenience, here is the v1 spec.

I can work on this later this week. Once I get it working for my operator I'll port the changes back upstream to Bonny. Don't take that as a staking of territory! If someone else wants to take it I won't feel hurt/sad/whatever, just let me know so that we don't duplicate effort. (as always time is limited and needs to be invested wisely)

FreedomBen avatar Feb 08 '22 05:02 FreedomBen

Hey @FreedomBen, did you get amywhere with this?

mruoss avatar Mar 16 '22 11:03 mruoss

@mruoss yes! My operator got re-prioritized for a couple of sprints due to unexpected important stuff, but I'm back on it as of yesterday. I've got most of the work done now, I'm just testing. Unless something unexpected happens, I think I'll have it ready for PR by the end of the week. If things went really well (which rarely happens to me in software lol) it might be ready for PR this evening (US timezone).

Do you need it now?

FreedomBen avatar Mar 16 '22 17:03 FreedomBen

oh, that's great! I'm keen to see your approach. Also feel free to open a "WIP" PR to start a discussion even if it's not ready to be merged...

mruoss avatar Mar 16 '22 19:03 mruoss

Getting very close :crossed_fingers:

FreedomBen avatar Mar 22 '22 17:03 FreedomBen

I'm currently stuck with this issue using later version of k8s, the manifest generated will not work, we need to provide the open api spec right? What about using something like https://github.com/open-api-spex/open_api_spex

danhawkins avatar Aug 15 '22 11:08 danhawkins

@danhawkins We need to provide the CRD in its new format, including the spec yes. open_api_spex is used for creating APIs, no? Can you elaborate in how this lib could help us? Also have a look at the discussions in #9.

@FreedomBen your last post sounded so promising! What happened?

mruoss avatar Aug 20 '22 15:08 mruoss

Any updates here? I'm also stuck in my Operator :(

sleipnir avatar Aug 26 '22 21:08 sleipnir

Hmm... it looks like Cory had already implemented something. Can anyone test this? What happens if you add this to your config.exs:

config :bonny,
  api_version: "apiextensions.k8s.io/v1"

Edit: See #101

mruoss avatar Aug 31 '22 10:08 mruoss

It would be nice ot there was a way to actually define the openapi schema...

mruoss avatar Aug 31 '22 10:08 mruoss

When use this option generated file use api_version specified in configuration. But without the proper schema we still can't apply the resources in kubernetes:

minikube start
😄  minikube v1.26.1 on Debian 11.0
✨  Using the docker driver based on existing profile
👍  Starting control plane node minikube in cluster minikube
🚜  Pulling base image ...
🔄  Restarting existing docker container for "minikube" ...
🐳  Preparing Kubernetes v1.24.3 on Docker 20.10.17 ...
🔎  Verifying Kubernetes components...
    ▪ Using image gcr.io/k8s-minikube/storage-provisioner:v5
🌟  Enabled addons: storage-provisioner, default-storageclass

❗  /usr/local/bin/kubectl is version 1.21.3, which may have incompatibilites with Kubernetes 1.24.3.
    ▪ Want kubectl v1.24.3? Try 'minikube kubectl -- get pods -A'
🏄  Done! kubectl is now configured to use "minikube" cluster and "default" namespace by default
sleipnir @ pop-os spawn main 
└─ $ (k8s: minikube) 🚀 ▶ make apply-k8s-manifests 
kubectl -n eigr-functions apply -f apps/operator/manifest.yaml
deployment.apps/eigr-functions configured
error: error validating "apps/operator/manifest.yaml": error validating data: [ValidationError(CustomResourceDefinition.spec.versions[0]): unknown field "scheme" in io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.CustomResourceDefinitionVersion, ValidationError(CustomResourceDefinition.spec.versions[0]): missing required field "served" in io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.CustomResourceDefinitionVersion, ValidationError(CustomResourceDefinition.spec.versions[0]): missing required field "storage" in io.k8s.apiextensions-apiserver.pkg.apis.apiextensions.v1.CustomResourceDefinitionVersion]; if you choose to ignore these errors, turn validation off with --validate=false
make: *** [Makefile:81: apply-k8s-manifests] Error 1

Yaml generated:

apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  labels:
    eigr_functions_protocol_major_version: '0'
    eigr_functions_protocol_minor_version: '1'
    proxy_name: spawn
    k8s-app: eigr-functions-controller
  name: actorsystems.spawn.eigr.io
spec:
  group: spawn.eigr.io
  names:
    kind: ActorSystem
    plural: actorsystems
    shortNames:
      - as
      - actorsys
      - actorsystem
      - actorsystems
      - system
    singular: actorsystem
  scope: Cluster
  versions:
    - name: v1
      scheme:
        openAPIV3Scheme:
          additionalPrinterColumns:
            - description: Storage type of the Actor System
              jsonPath: .spec.storage.type
              name: storage
              type: string
            - description: |-
                CreationTimestamp is a timestamp representing the server time when this object was created. It is not guaranteed to be set in happens-before order across separate operations. Clients may not set this value. It is represented in RFC3339 form and is in UTC.
                
                      Populated by the system. Read-only. Null for lists. More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#metadata
              jsonPath: .metadata.creationTimestamp
              name: Age
              type: date

sleipnir avatar Aug 31 '22 12:08 sleipnir

Oh boy... seems like typos in #101. What if you manually change

  • scheme to schema
  • openAPIV3Scheme to openAPIV3Schema

No... still not... additionalPrinterColumns should not be part of the schema... this looks all wrong (still)

mruoss avatar Aug 31 '22 12:08 mruoss

Oh boy... seems like typos in #101. What if you manually change

* `scheme` to `schema`

* `openAPIV3Scheme` to `openAPIV3Schema`

No... still not... additionalPrinterColumns should not be part of the schema... this looks all wrong (still)

Yes I know https://github.com/sleipnir/bonny/blob/811f50c60a8e41533ca562f4a8d3643b35dc1af3/lib/bonny/crd.ex#L172

sleipnir avatar Aug 31 '22 12:08 sleipnir

These are the mistakes now:

Error from server (Invalid): error when creating "apps/operator/manifest.yaml": CustomResourceDefinition.apiextensions.k8s.io "activators.spawn.eigr.io" is invalid: [spec.versions: Invalid value: []apiextensions.CustomResourceDefinitionVersion{apiextensions.CustomResourceDefinitionVersion{Name:"v1", Served:false, Storage:false, Deprecated:false, DeprecationWarning:(*string)(nil), Schema:(*apiextensions.CustomResourceValidation)(nil), Subresources:(*apiextensions.CustomResourceSubresources)(nil), AdditionalPrinterColumns:[]apiextensions.CustomResourceColumnDefinition(nil)}}: must have exactly one version marked as storage version, spec.validation.openAPIV3Schema.type: Required value: must not be empty at the root, status.storedVersions: Invalid value: []string(nil): must have at least one stored version]
Error from server (Invalid): error when creating "apps/operator/manifest.yaml": CustomResourceDefinition.apiextensions.k8s.io "actornodes.spawn.eigr.io" is invalid: [spec.versions: Invalid value: []apiextensions.CustomResourceDefinitionVersion{apiextensions.CustomResourceDefinitionVersion{Name:"v1", Served:false, Storage:false, Deprecated:false, DeprecationWarning:(*string)(nil), Schema:(*apiextensions.CustomResourceValidation)(nil), Subresources:(*apiextensions.CustomResourceSubresources)(nil), AdditionalPrinterColumns:[]apiextensions.CustomResourceColumnDefinition(nil)}}: must have exactly one version marked as storage version, spec.validation.openAPIV3Schema.type: Required value: must not be empty at the root, status.storedVersions: Invalid value: []string(nil): must have at least one stored version]
Error from server (Invalid): error when creating "apps/operator/manifest.yaml": CustomResourceDefinition.apiextensions.k8s.io "actorsystems.spawn.eigr.io" is invalid: [spec.versions: Invalid value: []apiextensions.CustomResourceDefinitionVersion{apiextensions.CustomResourceDefinitionVersion{Name:"v1", Served:false, Storage:false, Deprecated:false, DeprecationWarning:(*string)(nil), Schema:(*apiextensions.CustomResourceValidation)(nil), Subresources:(*apiextensions.CustomResourceSubresources)(nil), AdditionalPrinterColumns:[]apiextensions.CustomResourceColumnDefinition(nil)}}: must have exactly one version marked as storage version, spec.validation.openAPIV3Schema.type: Required value: must not be empty at the root, status.storedVersions: Invalid value: []string(nil): must have at least one stored version]

sleipnir avatar Aug 31 '22 12:08 sleipnir

Opened a pull request (#143) where I try to fix this. But we don't have integration tests here yet. Need to test that first.

mruoss avatar Aug 31 '22 12:08 mruoss

I think we created a PR to fix the same thing :D https://github.com/coryodaniel/bonny/pull/144

sleipnir avatar Aug 31 '22 12:08 sleipnir

yup. Closed yours ;)

Edit: as mine also addresses some other problems.

mruoss avatar Aug 31 '22 12:08 mruoss

What do you mean exactly?

Ignore my previous comment. I had taken a test wrong

sleipnir avatar Aug 31 '22 13:08 sleipnir

@mruoss The new version works accordingly. I think there's enough material to spawn a new version of bonny now wdyt?

sleipnir @ pop-os spawn main 
└─ $ (k8s: minikube) 🚀 ▶ make apply-k8s-manifests 
kubectl -n eigr-functions apply -f apps/operator/manifest.yaml --validate=false
deployment.apps/eigr-functions configured
customresourcedefinition.apiextensions.k8s.io/activators.spawn.eigr.io created
customresourcedefinition.apiextensions.k8s.io/actornodes.spawn.eigr.io created
customresourcedefinition.apiextensions.k8s.io/actorsystems.spawn.eigr.io created
clusterrole.rbac.authorization.k8s.io/eigr-functions unchanged
serviceaccount/eigr-functions unchanged
clusterrolebinding.rbac.authorization.k8s.io/eigr-functions unchanged
sleipnir @ pop-os spawn main 
└─ $ (k8s: minikube) 🚀 ▶ make apply-k8s-manifests 
kubectl -n eigr-functions apply -f apps/operator/manifest.yaml
deployment.apps/eigr-functions configured
customresourcedefinition.apiextensions.k8s.io/activators.spawn.eigr.io unchanged
customresourcedefinition.apiextensions.k8s.io/actornodes.spawn.eigr.io configured
customresourcedefinition.apiextensions.k8s.io/actorsystems.spawn.eigr.io unchanged
clusterrole.rbac.authorization.k8s.io/eigr-functions unchanged
serviceaccount/eigr-functions unchanged
clusterrolebinding.rbac.authorization.k8s.io/eigr-functions unchanged
sleipnir @ pop-os spawn main 
└─ $ (k8s: minikube) 🚀 ▶ k get activators
No resources found
sleipnir @ pop-os spawn main 
└─ $ (k8s: minikube) 🚀 ▶ k get actors
No resources found
sleipnir @ pop-os spawn main 
└─ $ (k8s: minikube) 🚀 ▶ k get bla
error: the server doesn't have a resource type "bla"
sleipnir @ pop-os spawn main 
└─ $ (k8s: minikube) 🚀 ▶ 

sleipnir avatar Aug 31 '22 13:08 sleipnir

For people looking for a solution, the generated CRD should work now (since version 0.5.2), once you have added the following entry to your config.exs:

config :bonny,
  api_version: "apiextensions.k8s.io/v1"

mruoss avatar Aug 31 '22 14:08 mruoss