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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions pkg/apis/networkaddonsoperator/shared/networkaddonsconfig_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,22 @@ type NetworkAddonsConfigSpec struct {
}

// +k8s:openapi-gen=true
// SelfSignConfiguration defines self sign configuration
type SelfSignConfiguration struct {
// CARotateInterval defines duration for CA and certificate
CARotateInterval string `json:"caRotateInterval,omitempty"`
// CAOverlapInterval defines the duration of CA Certificates at CABundle if not set it will default to CARotateInterval
CAOverlapInterval string `json:"caOverlapInterval,omitempty"`
// CertRotateInterval defines duration for of service certificate
CertRotateInterval string `json:"certRotateInterval,omitempty"`
}

// +k8s:openapi-gen=true
// PlacementConfiguration defines node placement configuration
type PlacementConfiguration struct {
// Infra defines placement configuration for master nodes
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

}

Expand All @@ -40,24 +47,32 @@ type Placement struct {
Tolerations []corev1.Toleration `json:"tolerations,omitempty"`
}

// Multus plugin enables attaching multiple network interfaces to Pods in Kubernetes
// +k8s:openapi-gen=true
type Multus struct{}

// LinuxBridge plugin allows users to create a bridge and add the host and the container to it
// +k8s:openapi-gen=true
type LinuxBridge struct{}

// Ovs plugin allows users to define Kubernetes networks on top of Open vSwitch bridges available on nodes
// +k8s:openapi-gen=true
type Ovs struct{}

// NMState is a declarative node network configuration driven through Kubernetes API
// +k8s:openapi-gen=true
type NMState struct{}

// KubeMacPool plugin manages MAC allocation to Pods and VMs in Kubernetes
// +k8s:openapi-gen=true
type KubeMacPool struct {
// RangeStart defines the first mac in range
RangeStart string `json:"rangeStart,omitempty"`
// RangeEnd defines the last mac in range
RangeEnd string `json:"rangeEnd,omitempty"`
}

// MacvtapCni plugin allows users to define Kubernetes networks on top of existing host interfaces
// +k8s:openapi-gen=true
type MacvtapCni struct{}

Expand All @@ -79,8 +94,7 @@ type Container struct {
}

// NetworkAddonsConfig is the Schema for the networkaddonsconfigs API
// This struct is no exposed/registered as part of the CRD, but is used
// by the v1alpha1 and v1 as kind of inside-helper struct
// This struct is no exposed/registered as part of the CRD, but is used by the v1alpha1 and v1 as kind of inside-helper struct
type NetworkAddonsConfig struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

// NetworkAddonsConfig is the Schema for the networkaddonsconfigs API
// +kubebuilder:subresource:status
// +kubebuilder:resource:path=networkaddonsconfigs,scope=Namespaced
// +kubebuilder:resource:path=networkaddonsconfigs,scope=Cluster
// +k8s:openapi-gen=true
type NetworkAddonsConfig struct {
metav1.TypeMeta `json:",inline"`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (

// NetworkAddonsConfig is the Schema for the networkaddonsconfigs API
// +kubebuilder:subresource:status
// +kubebuilder:resource:path=networkaddonsconfigs,scope=Namespaced
// +kubebuilder:resource:path=networkaddonsconfigs,scope=Cluster
// +k8s:openapi-gen=true
type NetworkAddonsConfig struct {
metav1.TypeMeta `json:",inline"`
Expand Down
82 changes: 70 additions & 12 deletions pkg/components/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,20 @@ func GetClusterRole() *rbacv1.ClusterRole {

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)

subResouceSchema := &extv1.CustomResourceSubresources{Status: &extv1.CustomResourceSubresourceStatus{}}
placementProps := map[string]extv1.JSONSchemaProps{
"NodeSelector" : extv1.JSONSchemaProps{
Type: "object",
},
"Affinity" : extv1.JSONSchemaProps{
Description: "Affinity is a group of affinity scheduling rules.",
Type: "object",
},
"Tolerations" : extv1.JSONSchemaProps{
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",
},
}

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.

OpenAPIV3Schema: &extv1.JSONSchemaProps{
Description: "NetworkAddonsConfig is the Schema for the networkaddonsconfigs API",
Expand All @@ -391,37 +405,80 @@ func GetCrd() *extv1.CustomResourceDefinition {
Type: "object",
},
"spec": extv1.JSONSchemaProps{
Type: "object",
Description: "NetworkAddonsConfigSpec defines the desired state of NetworkAddonsConfig",
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"imagePullPolicy": extv1.JSONSchemaProps{
Description: "PullPolicy describes a policy for if/when to pull a container image",
Type: "string",
},
"kubeMacPool": extv1.JSONSchemaProps{
Type: "object",
Description: "KubeMacPool plugin manages MAC allocation to Pods and VMs in Kubernetes",
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"rangeEnd": extv1.JSONSchemaProps{
Type: "string",
Description: "RangeEnd defines the first mac in range",
Type: "string",
},
"rangeStart": extv1.JSONSchemaProps{
Type: "string",
Description: "RangeStart defines the first mac in range",
Type: "string",
},
},
},
"linuxBridge": extv1.JSONSchemaProps{
Type: "object",
Description: "LinuxBridge plugin allows users to create a bridge and add the host and the container to it",
Type: "object",
},
"macvtap": extv1.JSONSchemaProps{
Type: "object",
Description: "MacvtapCni plugin allows users to define Kubernetes networks on top of existing host interfaces",
Type: "object",
},
"multus": extv1.JSONSchemaProps{
Type: "object",
Description: "Multus plugin enables attaching multiple network interfaces to Pods in Kubernetes",
Type: "object",
},
"nmstate": extv1.JSONSchemaProps{
Type: "object",
Description: "NMState is a declarative node network configuration driven through Kubernetes API",
Type: "object",
},
"ovs": extv1.JSONSchemaProps{
Description: "Ovs plugin allows users to define Kubernetes networks on top of Open vSwitch bridges available on nodes",
Type: "object",
},
"selfSignConfiguration": extv1.JSONSchemaProps{
Description: "SelfSignConfiguration defines self sign configuration",
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"caRotateInterval": extv1.JSONSchemaProps{
Description: "CARotateInterval defines duration for CA and certificate",
Type: "string",
},
"certRotateInterval": extv1.JSONSchemaProps{
Description: "CertRotateInterval defines duration for of service certificate",
Type: "string",
},
"caOverlapInterval": extv1.JSONSchemaProps{
Description: "CAOverlapInterval defines the duration of CA Certificates at CABundle if not set it will default to CARotateInterval",
Type: "string",
},
},
},
"PlacementConfiguration": extv1.JSONSchemaProps{
Description: "PlacementConfiguration defines node placement configuration",
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"Infra": extv1.JSONSchemaProps{
Description: "Infra defines placement configuration for master nodes",
Type: "object",
Properties: placementProps,
},
"Workloads": extv1.JSONSchemaProps{
Description: "Workloads defines placement configuration for worker nodes",
Type: "object",
Properties: placementProps,
},
},
},
},
},
Expand All @@ -430,19 +487,20 @@ func GetCrd() *extv1.CustomResourceDefinition {
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"conditions": extv1.JSONSchemaProps{
//Description: "Condition represents the state of the operator's reconciliation functionality.",
Type: "array",
Items: &extv1.JSONSchemaPropsOrArray{
Schema: &extv1.JSONSchemaProps{
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"lastHeartbeatTime": extv1.JSONSchemaProps{
Format: "date-time",
Type: "object",
Format: "date-time",
Type: "string",
Nullable: true,
},
"lastTransitionTime": extv1.JSONSchemaProps{
Format: "date-time",
Type: "object",
Format: "date-time",
Type: "string",
Nullable: true,
},
"message": extv1.JSONSchemaProps{
Expand Down
9 changes: 9 additions & 0 deletions test/check/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,15 @@ func CheckComponentsDeployment(components []Component) {
}
}

func CheckCrdExplainable() {
By("Checking crd is explainable")
explain, err := Kubectl("explain", "networkaddonsconfigs")
Expect(err).NotTo(HaveOccurred(), "explain should not return error")

Expect(explain).ToNot(BeEmpty(), "explain output should not be empty")
Expect(explain).ToNot(ContainSubstring("<empty>"), "explain output should not contain <empty> fields")
}

func CheckComponentsRemoval(components []Component) {
for _, component := range components {
if component.ComponentName == MultusComponent.ComponentName && IsOnOKDCluster() {
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/lifecycle/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ var _ = Context("Cluster Network Addons Operator", func() {
CheckConfigVersions(newReleaseGvk, expectedOperatorVersion, expectedObservedVersion, expectedTargetVersion, podsDeploymentTimeout, CheckDoNotRepeat)
})

It("Should be able to explain the crd object", func() {
CheckCrdExplainable()
})

It("it should report expected deployed container images and leave no leftovers from the previous version", func() {
By("Checking reported container images")
CheckReleaseUsesExpectedContainerImages(newReleaseGvk, newRelease)
Expand Down
7 changes: 7 additions & 0 deletions tools/manifest-templator/manifest-templator.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ func getCNA(data *templateData) {
crd := components.GetCrd()
marshallObject(crd, &writer)
crdString := writer.String()
crdString = addPreserveUnknownFields(crdString)
crdVersion := crd.Spec.Versions[0].Name

// Get CNA CR
Expand Down Expand Up @@ -224,6 +225,12 @@ func getCNA(data *templateData) {
data.CNA = &cnaData
}

func addPreserveUnknownFields(crdString string) string {
// TODO replace this solution with a better one once this issue get resolved:
// https://github.com/kubernetes/kubernetes/issues/95702
return crdString + " preserveUnknownFields: false"
}

func main() {
version := flag.String("version", "", "The csv version")
versionReplaces := flag.String("version-replaces", "", "The csv version this replaces")
Expand Down