bitpoke / stack

Open-Source WordPress Infrastructure on Kubernetes
https://www.bitpoke.io/stack
Apache License 2.0
162 stars 31 forks source link

x-kubernetes-list-map-keys on k8s 1.18 should be unique #104

Closed isnuryusuf closed 4 years ago

isnuryusuf commented 4 years ago

What happened: Deploying CRD on k8s version 1.18 fail with error:

[root@master-node linux-amd64]# kubectl apply -f 00-crds.yaml --validate=false
customresourcedefinition.apiextensions.k8s.io/mysqlclusters.mysql.presslabs.org configured
customresourcedefinition.apiextensions.k8s.io/mysqlbackups.mysql.presslabs.org configured
customresourcedefinition.apiextensions.k8s.io/issuers.cert-manager.io unchanged
customresourcedefinition.apiextensions.k8s.io/orders.acme.cert-manager.io unchanged
customresourcedefinition.apiextensions.k8s.io/certificaterequests.cert-manager.io unchanged
customresourcedefinition.apiextensions.k8s.io/certificates.cert-manager.io unchanged
customresourcedefinition.apiextensions.k8s.io/challenges.acme.cert-manager.io unchanged
customresourcedefinition.apiextensions.k8s.io/clusterissuers.cert-manager.io unchanged
customresourcedefinition.apiextensions.k8s.io/alertmanagers.monitoring.coreos.com unchanged
customresourcedefinition.apiextensions.k8s.io/podmonitors.monitoring.coreos.com unchanged
customresourcedefinition.apiextensions.k8s.io/prometheuses.monitoring.coreos.com unchanged
customresourcedefinition.apiextensions.k8s.io/prometheusrules.monitoring.coreos.com unchanged
customresourcedefinition.apiextensions.k8s.io/servicemonitors.monitoring.coreos.com unchanged
customresourcedefinition.apiextensions.k8s.io/thanosrulers.monitoring.coreos.com unchanged
The CustomResourceDefinition "wordpresses.wordpress.presslabs.org" is invalid: 
* spec.validation.openAPIV3Schema.properties[spec].properties[sidecars].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property
* spec.validation.openAPIV3Schema.properties[spec].properties[initContainers].items.properties[ports].items.properties[protocol].default: Required value: this property is in x-kubernetes-list-map-keys, so it must have a default or be a required property

What you expected to happen: CRD Deploy success

How to reproduce it (as minimally and precisely as possible):

[root@master-node linux-amd64]# kubectl version
Client Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.4", GitCommit:"c96aede7b5205121079932896c4ad89bb93260af", GitTreeState:"clean", BuildDate:"2020-06-17T11:41:22Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}
Server Version: version.Info{Major:"1", Minor:"18", GitVersion:"v1.18.4", GitCommit:"c96aede7b5205121079932896c4ad89bb93260af", GitTreeState:"clean", BuildDate:"2020-06-17T11:33:59Z", GoVersion:"go1.13.9", Compiler:"gc", Platform:"linux/amd64"}
[root@master-node linux-amd64]# docker version
Client:
 Version:         1.13.1
 API version:     1.26
 Package version: docker-1.13.1-161.git64e9980.el7_8.x86_64
 Go version:      go1.10.3
 Git commit:      64e9980/1.13.1
 Built:           Tue Apr 28 14:43:01 2020
 OS/Arch:         linux/amd64

Server:
 Version:         1.13.1
 API version:     1.26 (minimum version 1.12)
 Package version: docker-1.13.1-161.git64e9980.el7_8.x86_64
 Go version:      go1.10.3
 Git commit:      64e9980/1.13.1
 Built:           Tue Apr 28 14:43:01 2020
 OS/Arch:         linux/amd64
 Experimental:    false

kubectl apply -f 00-crds.yaml 

Anything else?: on https://kubernetes.io/docs/setup/release/notes/ Fixed missing validation of uniqueness of list items in lists with x-kubernetes-list-type: map or x-kubernetes-list-type: set` in CustomResources. (#84920, @sttts) [SIG API Machinery]

AMecea commented 4 years ago

Hi @isnuryusuf, thank you for this bug report.

I've investigated a little and this issue is rooted deeper. (https://github.com/kubernetes/kubernetes/issues/91395#issuecomment-633261523) We should update to apiextensions/v1 this will allow defaulting fields in validation schema. Meanwhile, controller-tools should find a way to generate those defaults because right now those are not set (https://github.com/kubernetes-sigs/controller-tools/issues/444).

So even if we will switch for apiextensions/v1 which requires k8s >= 1.16 this will not be fixed without a workaround.

My proposal is to remove validation for Container.Ports until this is fixed upstream, this is addressed in https://github.com/presslabs/wordpress-operator/pull/74.

sttts commented 4 years ago

https://github.com/kubernetes/kubernetes/pull/92659 should fix this assuming the project decides to accept kubebuilder markers in the API and, as you say, you switch to v1 CRDs.

proboxdev commented 4 years ago

Is this fixed now? Running

kubectl apply -f https://raw.githubusercontent.com/presslabs/stack/master/deploy/manifests/00-crds.yaml

gives me the same result as @isnuryusuf

isnuryusuf commented 4 years ago

@proboxdev not yet, but workaround has been mentioned above i decide to downgrade to v1.17