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

make it possible to run kubectl explain crd #524

Merged

Conversation

RamLavi
Copy link
Collaborator

@RamLavi RamLavi commented Sep 2, 2020

What this PR does / why we need it:
Let's make make it possible to run kubectl explain networkaddonsconfigs

  • add components annotations to types
    operator-sdk generate crds generates descriptions from the annotations added to networkaddonsconfig_types.go
    This commit adds these annotations.

  • fix NetworkAddonsConfig scope to Cluster in api types
    Currently there is a inconsistency between the manifest templator
    outputed crd manifest and the type annotations in scope field.
    Altough the manifests are controlled by the manifest generator
    lets fix the annotation avoid future confusion.

  • add openAPIV3Schema validation parameters
    Let's add openAPIV3Schema detail for "kubectl explain crd"
    Since operator-sdk crd cannot run. this commit manually creates the openAPIV3Schema
    that should've been generated by operator-sdk crd API.

  • adding test to make sure crd is explainable

Special notes for your reviewer:
PR depends on #537, #593 to be merged

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Sep 2, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Sep 2, 2020

output explain:

KIND:     NetworkAddonsConfig
VERSION:  networkaddonsoperator.network.kubevirt.io/v1

DESCRIPTION:
     NetworkAddonsConfig is the Schema for the networkaddonsconfigs API

FIELDS:
   apiVersion	<string>
     APIVersion defines the versioned schema of this representation of an
     object. Servers should convert recognized schemas to the latest internal
     value, and may reject unrecognized values. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources

   kind	<string>
     Kind is a string value representing the REST resource this object
     represents. Servers may infer this from the endpoint the client submits
     requests to. Cannot be updated. In CamelCase. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds

   metadata	<Object>
     Standard object's metadata. More info:
     https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#metadata

   spec	<Object>
     NetworkAddonsConfigSpec defines the desired state of NetworkAddonsConfig

   status	<Object>
     NetworkAddonsConfigStatus defines the observed state of NetworkAddonsConfig

CRD output:

---
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
metadata:
  name: networkaddonsconfigs.networkaddonsoperator.network.kubevirt.io
spec:
  group: networkaddonsoperator.network.kubevirt.io
  names:
    kind: NetworkAddonsConfig
    listKind: NetworkAddonsConfigList
    plural: networkaddonsconfigs
    singular: networkaddonsconfig
  scope: Cluster
  versions:
  - name: v1
    schema:
      openAPIV3Schema:
        description: NetworkAddonsConfig is the Schema for the networkaddonsconfigs
          API
        properties:
          apiVersion:
            description: 'APIVersion defines the versioned schema of this representation
              of an object. Servers should convert recognized schemas to the latest
              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
            type: string
          kind:
            description: 'Kind is a string value representing the REST resource this
              object represents. Servers may infer this from the endpoint the client
              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
            type: string
          metadata:
            type: object
          spec:
            description: NetworkAddonsConfigSpec defines the desired state of NetworkAddonsConfig
            properties:
              PlacementConfiguration:
                description: PlacementConfiguration defines node placement configuration
                properties:
                  Infra:
                    description: Infra defines placement configuration for master
                      nodes
                    properties:
                      Affinity:
                        description: Affinity is a group of affinity scheduling rules.
                        type: object
                      NodeSelector:
                        type: object
                      Tolerations:
                        description: The pod this Toleration is attached to tolerates
                          any taint that matches the triple <key,value,effect> using
                          the matching operator <operator>.
                        type: object
                    type: object
                  Workloads:
                    description: Workloads defines placement configuration for worker
                      nodes
                    properties:
                      Affinity:
                        description: Affinity is a group of affinity scheduling rules.
                        type: object
                      NodeSelector:
                        type: object
                      Tolerations:
                        description: The pod this Toleration is attached to tolerates
                          any taint that matches the triple <key,value,effect> using
                          the matching operator <operator>.
                        type: object
                    type: object
                type: object
              imagePullPolicy:
                description: PullPolicy describes a policy for if/when to pull a container
                  image
                type: string
              kubeMacPool:
                description: KubeMacPool plugin manages MAC allocation to Pods and
                  VMs in Kubernetes
                properties:
                  rangeEnd:
                    description: Defines the last mac in range
                    type: string
                  rangeStart:
                    description: Defines the first mac in range
                    type: string
                type: object
              linuxBridge:
                description: LinuxBridge plugin allows users to create a bridge and
                  add the host and the container to it
                type: object
              macvtap:
                description: MacvtapCni plugin allows users to define Kubernetes networks
                  on top of existing host interfaces
                type: object
              multus:
                description: Multus plugin enables attaching multiple network interfaces
                  to Pods in Kubernetes
                type: object
              nmstate:
                description: NMState is a declarative node network configuration driven
                  through Kubernetes API
                type: object
              ovs:
                description: Ovs plugin allows users to define Kubernetes networks
                  on top of Open vSwitch bridges available on nodes
                type: object
              selfSignConfiguration:
                description: SelfSignConfiguration defines self sign configuration
                properties:
                  caOverlapInterval:
                    description: CAOverlapInterval defines the duration of CA Certificates
                      at CABundle if not set it will default to CARotateInterval
                    type: string
                  caRotateInterval:
                    description: CARotateInterval defines duration for CA and certificate
                    type: string
                  certRotateInterval:
                    description: CertRotateInterval defines duration for of service
                      certificate
                    type: string
                type: object
            type: object
          status:
            description: NetworkAddonsConfigStatus defines the observed state of NetworkAddonsConfig
            properties:
              conditions:
                items:
                  properties:
                    lastHeartbeatTime:
                      format: date-time
                      type: string
                    lastTransitionTime:
                      format: date-time
                      type: string
                    message:
                      type: string
                    reason:
                      type: string
                    status:
                      type: string
                    type:
                      description: ConditionType is the state of the operator's reconciliation
                        functionality.
                      type: string
                  required:
                  - status
                  - type
                  type: object
                type: array
              containers:
                items:
                  properties:
                    image:
                      type: string
                    name:
                      type: string
                    parentKind:
                      type: string
                    parentName:
                      type: string
                  required:
                  - image
                  - name
                  - parentKind
                  - parentName
                  type: object
                type: array
              observedVersion:
                type: string
              operatorVersion:
                type: string
              targetVersion:
                type: string
            type: object
        type: object
    served: true
    storage: true
    subresources:
      status: {}
  - name: v1alpha1
    schema:
      openAPIV3Schema:
        description: NetworkAddonsConfig is the Schema for the networkaddonsconfigs
          API
        properties:
          apiVersion:
            description: 'APIVersion defines the versioned schema of this representation
              of an object. Servers should convert recognized schemas to the latest
              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
            type: string
          kind:
            description: 'Kind is a string value representing the REST resource this
              object represents. Servers may infer this from the endpoint the client
              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
            type: string
          metadata:
            type: object
          spec:
            description: NetworkAddonsConfigSpec defines the desired state of NetworkAddonsConfig
            properties:
              PlacementConfiguration:
                description: PlacementConfiguration defines node placement configuration
                properties:
                  Infra:
                    description: Infra defines placement configuration for master
                      nodes
                    properties:
                      Affinity:
                        description: Affinity is a group of affinity scheduling rules.
                        type: object
                      NodeSelector:
                        type: object
                      Tolerations:
                        description: The pod this Toleration is attached to tolerates
                          any taint that matches the triple <key,value,effect> using
                          the matching operator <operator>.
                        type: object
                    type: object
                  Workloads:
                    description: Workloads defines placement configuration for worker
                      nodes
                    properties:
                      Affinity:
                        description: Affinity is a group of affinity scheduling rules.
                        type: object
                      NodeSelector:
                        type: object
                      Tolerations:
                        description: The pod this Toleration is attached to tolerates
                          any taint that matches the triple <key,value,effect> using
                          the matching operator <operator>.
                        type: object
                    type: object
                type: object
              imagePullPolicy:
                description: PullPolicy describes a policy for if/when to pull a container
                  image
                type: string
              kubeMacPool:
                description: KubeMacPool plugin manages MAC allocation to Pods and
                  VMs in Kubernetes
                properties:
                  rangeEnd:
                    description: Defines the last mac in range
                    type: string
                  rangeStart:
                    description: Defines the first mac in range
                    type: string
                type: object
              linuxBridge:
                description: LinuxBridge plugin allows users to create a bridge and
                  add the host and the container to it
                type: object
              macvtap:
                description: MacvtapCni plugin allows users to define Kubernetes networks
                  on top of existing host interfaces
                type: object
              multus:
                description: Multus plugin enables attaching multiple network interfaces
                  to Pods in Kubernetes
                type: object
              nmstate:
                description: NMState is a declarative node network configuration driven
                  through Kubernetes API
                type: object
              ovs:
                description: Ovs plugin allows users to define Kubernetes networks
                  on top of Open vSwitch bridges available on nodes
                type: object
              selfSignConfiguration:
                description: SelfSignConfiguration defines self sign configuration
                properties:
                  caOverlapInterval:
                    description: CAOverlapInterval defines the duration of CA Certificates
                      at CABundle if not set it will default to CARotateInterval
                    type: string
                  caRotateInterval:
                    description: CARotateInterval defines duration for CA and certificate
                    type: string
                  certRotateInterval:
                    description: CertRotateInterval defines duration for of service
                      certificate
                    type: string
                type: object
            type: object
          status:
            description: NetworkAddonsConfigStatus defines the observed state of NetworkAddonsConfig
            properties:
              conditions:
                items:
                  properties:
                    lastHeartbeatTime:
                      format: date-time
                      type: string
                    lastTransitionTime:
                      format: date-time
                      type: string
                    message:
                      type: string
                    reason:
                      type: string
                    status:
                      type: string
                    type:
                      description: ConditionType is the state of the operator's reconciliation
                        functionality.
                      type: string
                  required:
                  - status
                  - type
                  type: object
                type: array
              containers:
                items:
                  properties:
                    image:
                      type: string
                    name:
                      type: string
                    parentKind:
                      type: string
                    parentName:
                      type: string
                  required:
                  - image
                  - name
                  - parentKind
                  - parentName
                  type: object
                type: array
              observedVersion:
                type: string
              operatorVersion:
                type: string
              targetVersion:
                type: string
            type: object
        type: object
    served: true
    storage: false
    subresources:
      status: {}

@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 2fddf5f to 5a87814 Compare September 2, 2020 16:43
@RamLavi RamLavi changed the title add openAPIV3Schema validation parameters make make it possible to run kubectl explain crd Sep 2, 2020
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 5a87814 to 35618d9 Compare September 2, 2020 16:55
@RamLavi RamLavi changed the title make make it possible to run kubectl explain crd make it possible to run kubectl explain crd Sep 2, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Sep 3, 2020

getting Failed calling status Set 8/10: Failed to update NetworkAddonsConfig "cluster" Status: networkaddonsconfigs.networkaddonsoperator.network.kubevirt.io "cluster" not found although :

kubectl get networkaddonsconfigs
NAME      AGE
cluster   60s

@@ -368,63 +368,198 @@ func GetClusterRole() *rbacv1.ClusterRole {
return role
}

func GetCrd() *extv1beta1.CustomResourceDefinition {
crd := &extv1beta1.CustomResourceDefinition{
func GetCrd() *extv1.CustomResourceDefinition {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't we want to annotate the _types with descriptions and generate this with gen-openapi ?

Copy link
Collaborator Author

@RamLavi RamLavi Sep 3, 2020

Choose a reason for hiding this comment

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

the existence of the /shared folder in types cause an error with operator-sdk generate crds command.
so, I created it in an older version for reference and then recreated it manually in components.go:GetCrd().
the resulting .crd file is identical to the reference (except for the version migration changes of course)

@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch 2 times, most recently from 4e281ba to d7b29f8 Compare September 6, 2020 12:11
@RamLavi RamLavi mentioned this pull request Sep 6, 2020
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch 3 times, most recently from 73779c2 to 51557ba Compare September 8, 2020 10:39
@RamLavi
Copy link
Collaborator Author

RamLavi commented Sep 8, 2020

updated crd and explain outputs

Infra *Placement `json:"infra,omitempty"`
// Workloads defines placement configuration for worker nodes
Workloads *Placement `json:"workloads,omitempty"`
Copy link
Collaborator

@qinqon qinqon Sep 8, 2020

Choose a reason for hiding this comment

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

Placement struct is missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

func GetCrd() *extv1beta1.CustomResourceDefinition {
crd := &extv1beta1.CustomResourceDefinition{
func GetCrd() *extv1.CustomResourceDefinition {
validationSchema := &extv1.CustomResourceValidation{
Copy link
Collaborator

@qinqon qinqon Sep 8, 2020

Choose a reason for hiding this comment

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

Add we sure we cannot store at the container the stuff generated from operator-sdk and read it as golang struct ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we currently can't generate it, it doesn't know how to handle the shared folder:

cluster-network-addons-operator (add_csv_schema_validation_detail) $ ./build/_output/bin/operator-sdk generate crds
INFO[0000] Running CRD generator.                       

github.com/kubevirt/cluster-network-addons-operator/pkg/apis/networkaddonsoperator/shared:-: CRD for NetworkAddonsConfig.networkaddonsoperator.network.kubevirt.io has no storage version
FATA[0001] error generating CRDs from APIs in pkg/apis: error generating CRD manifests: error running CRD generator: not all generators ran successfully 

plus, (correct me if I'm wrong, but) marshaling the automated yaml to struct will require us to always know the exact struct format. so in fact it could mean taking up the same amount of time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well it's all about duplicity I see the description on structs and on the Schema, It has to be doable somehow we will se in the future if we do deeper integration of cnao with operator-sdk.

@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 173867c to 5b3a350 Compare September 8, 2020 13:33
@RamLavi
Copy link
Collaborator Author

RamLavi commented Sep 8, 2020

updated crd output
changed in this push

@qinqon
Copy link
Collaborator

qinqon commented Sep 8, 2020

/lgtm
/approve

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 8, 2020
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 5b3a350 to f917f7e Compare September 8, 2020 17:03
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Sep 8, 2020
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from f917f7e to 542e5c6 Compare September 8, 2020 17:10
@@ -32,6 +33,7 @@ var _ = Context("Cluster Network Addons Operator", func() {
BeforeEach(func() {
UninstallRelease(oldRelease)
InstallRelease(newRelease)
CheckCrdExplainable()
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to keep the second one in It? Since it is really something we want to test, not a setup of the context

@@ -18,6 +18,7 @@ var _ = Context("Cluster Network Addons Operator", func() {
Context(fmt.Sprintf("when operator in version %s is installed and supported spec configured", oldRelease.Version), func() {
BeforeEach(func() {
InstallRelease(oldRelease)
CheckCrdExplainable()
Copy link
Member

Choose a reason for hiding this comment

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

Why do we care about this in before each?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the idea was to check before and after the upgrade.
I agree this would better suited in a specific It. tell me what you think
Done

@RamLavi
Copy link
Collaborator Author

RamLavi commented Sep 30, 2020

/hold

@qinqon is working on a PR to auto generate the crd via operator-sdk

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 30, 2020
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 15b8146 to c60d39e Compare September 30, 2020 06:26
@RamLavi
Copy link
Collaborator Author

RamLavi commented Oct 5, 2020

both failed test don't recreate locally, retesting

/retest

@qinqon
Copy link
Collaborator

qinqon commented Oct 5, 2020

/hold cancel

We are not going to depend on auto generated CRDs since looks like we have some non tooling issues there.

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2020
@qinqon
Copy link
Collaborator

qinqon commented Oct 5, 2020

/lgtm
/approve

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 5, 2020
@kubevirt-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: qinqon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Oct 5, 2020
@qinqon
Copy link
Collaborator

qinqon commented Oct 5, 2020

/hold

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 5, 2020
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from a4ecb32 to 0639214 Compare October 5, 2020 12:30
@RamLavi
Copy link
Collaborator Author

RamLavi commented Oct 11, 2020

/retest

@RamLavi
Copy link
Collaborator Author

RamLavi commented Oct 11, 2020

crd explain fails since kubectl explain networkaddonsconfigs stays after upgrade of crd.
When running after crd creation (not upgraded from old version) - explain looks OK.
tried to change upgrade to use --force,--prune flags but they don't refresh the explain value.

'operator-sdk generate crds' generates descriptions from the annotations added to 'networkaddonsconfig_types.go'
This commit adds these annotations.

Signed-off-by: Ram Lavi <[email protected]>
Currently there is a inconsistency between the manifest templator
outputed crd manifest and the type annotations in scope field.
Altough the manifests are controlled by the manifest generator
lets fix the annotation avoid future confusion.

Signed-off-by: Ram Lavi <[email protected]>
Let's add openAPIV3Schema detail for "kubectl explain crd"
Since operator-sdk crd cannot run. this commit manually creates the openAPIV3Schema
that should've been generated by operator-sdk crd API.

Signed-off-by: Ram Lavi <[email protected]>
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 0639214 to 61715ae Compare October 15, 2020 14:23
@RamLavi
Copy link
Collaborator Author

RamLavi commented Oct 15, 2020

/retest

@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 61715ae to 523e9be Compare October 19, 2020 12:52
there is an issue where on upgrade of apiVersion from
apiextensions.k8s.io/v1beta1 to apiextensions.k8s.io/v1,
oc explain does not show the CR documentation.
This is solved in parallel PRs (SSP PR [1], CDI PR [2])
by adding PreserveUnknownFields=false to the CRD's spec

[1] kubevirt/kubevirt-ssp-operator@7ecd297
[2] kubevirt/containerized-data-importer#1343

Signed-off-by: Ram Lavi <[email protected]>
@RamLavi RamLavi force-pushed the add_csv_schema_validation_detail branch from 523e9be to dcc4418 Compare October 19, 2020 12:59
@RamLavi
Copy link
Collaborator Author

RamLavi commented Oct 19, 2020

/hold

awaiting response from kubernetes/kubernetes#95702 before merging

@qinqon
Copy link
Collaborator

qinqon commented Oct 20, 2020

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2020
@RamLavi
Copy link
Collaborator Author

RamLavi commented Oct 20, 2020

/hold cancel

since currently the parameter is not deprecated we can move on with the PR

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 20, 2020
@kubevirt-bot kubevirt-bot merged commit 2a0a2d4 into kubevirt:master Oct 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants