Skip to content

Commit

Permalink
Fix types.go vs code schema verification to actually fail if they are…
Browse files Browse the repository at this point in the history 
… different. (kubevirt#1428)

Signed-off-by: Alexander Wels <[email protected]>
  • Loading branch information
@awels
awels authored Oct 14, 2020
1 parent c0798bb commit 6046b73
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 57 deletions.
6 changes: 3 additions & 3 deletions hack/verify-codegen.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ go build -o ./bin/schema-exporter ./tools/schema-exporter

curl https://github.com/mikefarah/yq/releases/download/3.3.2/yq_linux_amd64 -L --output ./bin/yq
chmod +x ./bin/yq
./bin/yq compare _out/manifests/schema/cdi.kubevirt.io_cdiconfigs.yaml _out/manifests/code_schema/cdiconfigs.cdi.kubevirt.io spec || echo "CDIConfg crd schema does not match"
./bin/yq compare _out/manifests/schema/cdi.kubevirt.io_cdis.yaml _out/manifests/code_schema/cdis.cdi.kubevirt.io spec || echo "CDI crd schema does not match"
./bin/yq compare _out/manifests/schema/cdi.kubevirt.io_datavolumes.yaml _out/manifests/code_schema/datavolumes.cdi.kubevirt.io spec || echo "Datavolume crd schema does not match"
./bin/yq compare _out/manifests/schema/cdi.kubevirt.io_cdiconfigs.yaml _out/manifests/code_schema/cdiconfigs.cdi.kubevirt.io spec || (echo "CDIConfg crd schema does not match" && exit 1)
./bin/yq compare _out/manifests/schema/cdi.kubevirt.io_cdis.yaml _out/manifests/code_schema/cdis.cdi.kubevirt.io spec || (echo "CDI crd schema does not match" && exit 1)
./bin/yq compare _out/manifests/schema/cdi.kubevirt.io_datavolumes.yaml _out/manifests/code_schema/datavolumes.cdi.kubevirt.io spec || (echo "Datavolume crd schema does not match" && exit 1)
1 change: 0 additions & 1 deletion pkg/apis/core/v1alpha1/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ go_library(
deps = [
"//pkg/apis/core:go_default_library",
"//vendor/github.com/go-openapi/spec:go_default_library",
"//vendor/github.com/openshift/custom-resource-status/conditions/v1:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
Expand Down
5 changes: 2 additions & 3 deletions pkg/apis/core/v1alpha1/openapi_generated.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 1 addition & 12 deletions pkg/apis/core/v1alpha1/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
sdkapi "kubevirt.io/controller-lifecycle-operator-sdk/pkg/sdk/api"

conditions "github.com/openshift/custom-resource-status/conditions/v1"
)

// DataVolume is an abstraction on top of PersistentVolumeClaims to allow easy population of those PersistentVolumeClaims with relation to VirtualMachines
Expand Down Expand Up @@ -288,16 +286,7 @@ type CDIPhase string

// CDIStatus defines the status of the installation
type CDIStatus struct {
// Phase is the current phase of the deployment
Phase CDIPhase `json:"phase,omitempty"`
// A list of current conditions of the resource
Conditions []conditions.Condition `json:"conditions,omitempty" optional:"true"`
// The version of the resource as defined by the operator
OperatorVersion string `json:"operatorVersion,omitempty" optional:"true"`
// The desired version of the resource
TargetVersion string `json:"targetVersion,omitempty" optional:"true"`
// The observed version of the resource
ObservedVersion string `json:"observedVersion,omitempty" optional:"true"`
sdkapi.Status `json:",inline"`
}

const (
Expand Down
7 changes: 1 addition & 6 deletions pkg/apis/core/v1alpha1/types_swagger_generated.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 1 addition & 8 deletions pkg/apis/core/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 16 additions & 8 deletions pkg/operator/resources/cluster/cdiconfig.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,16 @@ func createCDIConfigCRD() *extv1.CustomResourceDefinition {
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"global": {
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (for example, a per storageClass value)",
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (per storageClass)",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
},
"storageClass": {
AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
Schema: &extv1.JSONSchemaProps{
Type: "string",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
Description: "Percent is a string that can only be a value between [0,1) (Note: we actually rely on reconcile to reject invalid values)",
},
},
Description: "StorageClass specifies how much space of a Filesystem volume should be reserved for safety. The keys are the storageClass and the values are the overhead. This value overrides the global value",
Expand Down Expand Up @@ -189,14 +191,16 @@ func createCDIConfigCRD() *extv1.CustomResourceDefinition {
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"global": {
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (for example, a per storageClass value)",
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (per storageClass)",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
},
"storageClass": {
AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
Schema: &extv1.JSONSchemaProps{
Type: "string",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
Description: "Percent is a string that can only be a value between [0,1) (Note: we actually rely on reconcile to reject invalid values)",
},
},
Description: "StorageClass specifies how much space of a Filesystem volume should be reserved for safety. The keys are the storageClass and the values are the overhead. This value overrides the global value",
Expand Down Expand Up @@ -301,14 +305,16 @@ func createCDIConfigCRD() *extv1.CustomResourceDefinition {
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"global": {
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (for example, a per storageClass value)",
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (per storageClass)",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
},
"storageClass": {
AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
Schema: &extv1.JSONSchemaProps{
Type: "string",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
Description: "Percent is a string that can only be a value between [0,1) (Note: we actually rely on reconcile to reject invalid values)",
},
},
Description: "StorageClass specifies how much space of a Filesystem volume should be reserved for safety. The keys are the storageClass and the values are the overhead. This value overrides the global value",
Expand Down Expand Up @@ -377,14 +383,16 @@ func createCDIConfigCRD() *extv1.CustomResourceDefinition {
Type: "object",
Properties: map[string]extv1.JSONSchemaProps{
"global": {
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (for example, a per storageClass value)",
Description: "Global is how much space of a Filesystem volume should be reserved for overhead. This value is used unless overridden by a more specific value (per storageClass)",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
},
"storageClass": {
AdditionalProperties: &extv1.JSONSchemaPropsOrBool{
Schema: &extv1.JSONSchemaProps{
Type: "string",
Type: "string",
Pattern: `^(0(?:\.\d{1,3})?|1)$`,
Description: "Percent is a string that can only be a value between [0,1) (Note: we actually rely on reconcile to reject invalid values)",
},
},
Description: "StorageClass specifies how much space of a Filesystem volume should be reserved for safety. The keys are the storageClass and the values are the overhead. This value overrides the global value",
Expand Down
35 changes: 19 additions & 16 deletions tests/cdiconfig_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -537,22 +537,25 @@ var _ = Describe("Modifying CDIConfig spec tests", func() {
Entry("zero2", "0.0", true),
)

DescribeTable("Should not update status if per-storageClass filesystem overhead value is invalid", func(overhead string, success bool) {
DescribeTable("Per-storageClass filesystem overhead value", func(overhead string, success bool) {
defaultSCName := utils.DefaultStorageClass.GetName()
config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
config.Spec.FilesystemOverhead = &cdiv1.FilesystemOverhead{
Global: "0.99", // Used to easily test that the update happened
StorageClass: map[string]cdiv1.Percent{defaultSCName: cdiv1.Percent(overhead)},
}
_, err = f.CdiClient.CdiV1beta1().CDIConfigs().Update(context.TODO(), config, metav1.UpdateOptions{})
Expect(err).ToNot(HaveOccurred())

By("Waiting for the CDIConfig status to be updated by the controller")
Eventually(func() bool {
config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
if success {
Expect(err).ToNot(HaveOccurred())
return config.Status.FilesystemOverhead.Global == cdiv1.Percent("0.99")
}, timeout, pollingInterval).Should(BeTrue(), "CDIConfig not set")
By("Waiting for the CDIConfig status to be updated by the controller")
Eventually(func() bool {
config, err := f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred())
return config.Status.FilesystemOverhead.Global == cdiv1.Percent("0.99")
}, timeout, pollingInterval).Should(BeTrue(), "CDIConfig not set")
} else {
Expect(err).To(HaveOccurred())
}

config, err = f.CdiClient.CdiV1beta1().CDIConfigs().Get(context.TODO(), common.ConfigName, metav1.GetOptions{})
if success {
Expand All @@ -563,14 +566,14 @@ var _ = Describe("Modifying CDIConfig spec tests", func() {
Expect(config.Status.FilesystemOverhead.StorageClass[defaultSCName]).ToNot(Equal(cdiv1.Percent(overhead)))
}
},
Entry("[test_id:5014] Not a number1", "abc", false),
Entry("[test_id:5015] Not a number2", "1.abc", false),
Entry("[test_id:5016] Too big1", "1.01", false),
Entry("[test_id:5017] Too big2", "inf", false),
Entry("[test_id:5918] Negative", "-0.1", false),
Entry("one", "1", true),
Entry("zero", "0", true),
Entry("zero2", "0.0", true),
Entry("[test_id:5014]should not update if not a number1", "abc", false),
Entry("[test_id:5015]should not update if not a number2", "1.abc", false),
Entry("[test_id:5016]should not update if too big1", "1.01", false),
Entry("[test_id:5017]should not update if too big2", "inf", false),
Entry("[test_id:5918]should not update if value is negative", "-0.1", false),
Entry("should update if value is one", "1", true),
Entry("should update if value is zero", "0", true),
Entry("should update if value is zero2", "0.0", true),
)

})
Expand Down

0 comments on commit 6046b73

Please sign in to comment.