wip: last fix before closing

Author: exdx

pkg/controller/operators/catalog/operator.go

@@ -1436,7 +1436,7 @@ func (o *Operator) ExecutePlan(plan *v1alpha1.InstallPlan) error {
 	}
 
 	ensurer := newStepEnsurer(kubeclient, crclient, dynamicClient)
-	b := newBuilder(kubeclient, dynamicClient, o.csvProvidedAPIsIndexer)
+	b := newBuilder(kubeclient, dynamicClient, o.csvProvidedAPIsIndexer, o.logger)
 
 	for i, step := range plan.Status.Plan {
 		doStep := true

pkg/controller/operators/catalog/step.go

@@ -36,13 +36,16 @@ type builder struct {
 	opclient          operatorclient.ClientInterface
 	dynamicClient     dynamic.Interface
 	csvToProvidedAPIs map[string]cache.Indexer
+	logger            logger.FieldLogger
 }
 
-func newBuilder(opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, csvToProvidedAPIs map[string]cache.Indexer) *builder {
+func newBuilder(opclient operatorclient.ClientInterface, dynamicClient dynamic.Interface, csvToProvidedAPIs map[string]cache.Indexer,
+	logger logger.FieldLogger) *builder {
 	return &builder{
 		opclient:          opclient,
 		dynamicClient:     dynamicClient,
 		csvToProvidedAPIs: csvToProvidedAPIs,
+		logger:            logger,
 	}
 }
 
@@ -106,20 +109,21 @@ func (b *builder) NewCRDStep(manifest string, client clientset.Interface, status
 			if err != nil {
 				return v1alpha1.StepStatusUnknown, err
 			}
-			logger.Debugf("crd version %s", version)
+			b.logger.Debugf("crd version %s", version)
 			if version == crdlib.V1Version {
 				crd, err := crdlib.SerializeV1(manifest)
 				if err != nil {
 					return v1alpha1.StepStatusUnknown, err
 				}
+				b.logger.Debugf("creating v1 CRD %#v", crd)
 				_, createError = client.ApiextensionsV1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
 				crdi = crd
 			} else if version == crdlib.V1Beta1Version {
 				crd, err := crdlib.SerializeV1Beta1(manifest)
 				if err != nil {
 					return v1alpha1.StepStatusUnknown, err
 				}
-				logger.Debugf("creating CRD %v", crdi)
+				b.logger.Debugf("creating v1beta1 CRD %#v", crd)
 				_, createError = client.ApiextensionsV1beta1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
 				crdi = crd
 			} else {
@@ -128,15 +132,19 @@ func (b *builder) NewCRDStep(manifest string, client clientset.Interface, status
 
 			// convert to v1 type for remaining CRD reconciliation logic
 			crd, err := crdlib.SerializeV1FromExisting(crdi)
-			logger.Debugf("converted crd %v", crd)
+			b.logger.Debugf("converted crd %#v", crd)
 			if err != nil {
 				return v1alpha1.StepStatusUnknown, fmt.Errorf("unable to serialize into v1 CRD from existing manifest: %s", err)
 			}
 
+			b.logger.Debugf("create error: %s", createError)
+
 			if k8serrors.IsAlreadyExists(createError) {
 				currentCRD, _ := client.ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), crd.GetName(), metav1.GetOptions{})
-				// Compare 2 CRDs to see if it needs to be updatetd
+				// Compare 2 CRDs to see if it needs to be updated
+				logger.Infof("\n current crd: %#v \n new crd: %#v \n", currentCRD, crd)
 				if crdlib.NotEqual(currentCRD, crd) {
+					b.logger.Debugf("not equal")
 					// Verify CRD ownership, only attempt to update if
 					// CRD has only one owner
 					// Example: provided=database.coreos.com/v1alpha1/EtcdCluster
@@ -180,7 +188,7 @@ func (b *builder) NewCRDStep(manifest string, client clientset.Interface, status
 				return v1alpha1.StepStatusPresent, nil
 			} else if createError != nil {
 				// Unexpected error creating the CRD.
-				return v1alpha1.StepStatusUnknown, err
+				return v1alpha1.StepStatusUnknown, createError
 			}
 			// If no error occured, make sure to wait for the API to become available.
 			return v1alpha1.StepStatusWaitingForAPI, nil

pkg/lib/crd/serialize.go

@@ -3,12 +3,10 @@ package crd
 import (
 	"bytes"
 	"fmt"
-	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install"
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/yaml"
 )
@@ -30,59 +28,29 @@ func init() {
 // SerializeV1 takes in a CRD manifest and returns a v1 versioned CRD object.
 // Compatible with v1beta1 or v1 CRD manifests.
 func SerializeV1(manifest string) (*apiextensionsv1.CustomResourceDefinition, error) {
-	u := &unstructured.Unstructured{}
+	crd := &apiextensionsv1.CustomResourceDefinition{}
 	reader := bytes.NewReader([]byte(manifest))
 	decoder := yaml.NewYAMLOrJSONDecoder(reader, 30)
-	if err := decoder.Decode(u); err != nil {
-		return nil, fmt.Errorf("crd unmarshaling failed: %s", err)
-	}
-
-	// Step through unversioned type to support v1beta1 -> v1
-	unversioned := &apiextensions.CustomResourceDefinition{}
-	if err := scheme.Convert(u, unversioned, nil); err != nil {
-		return nil, fmt.Errorf("failed to convert crd from unstructured to internal: %s\nto v1: %s", u, err)
-	}
-
-	crd := &apiextensionsv1.CustomResourceDefinition{}
-	if err := scheme.Convert(unversioned, crd, nil); err != nil {
-		return nil, fmt.Errorf("failed to convert crd from internal to v1: %s\nto v1: %s", u, err)
+	if err := decoder.Decode(crd); err != nil {
+		return nil, fmt.Errorf("v1 crd unmarshaling failed: %s", err)
 	}
 
 	// set CRD type meta
 	// for purposes of fake client for unit tests to pass
 	crd.TypeMeta.Kind = Kind
 	crd.TypeMeta.APIVersion = Group + V1Version
 
-	// for each version in the CRD, check and make sure there is a schema
-	// if not a schema, give a default schema of props
-	for i := range crd.Spec.Versions {
-		if crd.Spec.Versions[i].Schema == nil {
-			schema := &apiextensionsv1.JSONSchemaProps{Type: "object"}
-			crd.Spec.Versions[i].Schema = &apiextensionsv1.CustomResourceValidation{OpenAPIV3Schema: schema}
-		}
-	}
-
 	return crd, nil
 }
 
 // SerializeV1 takes in a CRD manifest and returns a v1 versioned CRD object.
 // Compatible with v1beta1 or v1 CRD manifests.
 func SerializeV1Beta1(manifest string) (*apiextensionsv1beta1.CustomResourceDefinition, error) {
-	u := &unstructured.Unstructured{}
+	crd := &apiextensionsv1beta1.CustomResourceDefinition{}
 	reader := bytes.NewReader([]byte(manifest))
 	decoder := yaml.NewYAMLOrJSONDecoder(reader, 30)
-	if err := decoder.Decode(u); err != nil {
-		return nil, fmt.Errorf("crd unmarshaling failed: %s", err)
-	}
-
-	unversioned := &apiextensions.CustomResourceDefinition{}
-	if err := scheme.Convert(u, unversioned, nil); err != nil {
-		return nil, fmt.Errorf("failed to convert crd from unstructured to internal: %s\nto v1: %s", u, err)
-	}
-
-	crd := &apiextensionsv1beta1.CustomResourceDefinition{}
-	if err := scheme.Convert(unversioned, crd, nil); err != nil {
-		return nil, fmt.Errorf("failed to convert crd from internal to v1: %s\nto v1: %s", u, err)
+	if err := decoder.Decode(crd); err != nil {
+		return nil, fmt.Errorf("v1beta1 crd unmarshaling failed: %s", err)
 	}
 
 	// set CRD type meta
@@ -99,16 +67,11 @@ func SerializeV1FromExisting(crd interface{}) (*apiextensionsv1.CustomResourceDe
 		return c, nil
 	}
 	if c, ok := crd.(*apiextensionsv1beta1.CustomResourceDefinition); ok {
-		unversioned := &apiextensions.CustomResourceDefinition{}
-		if err := scheme.Convert(c, unversioned, nil); err != nil {
-			return nil, fmt.Errorf("failed to convert crd from unstructured to internal: %s\nto v1: %s", c.String(), err)
-		}
-
-		c := &apiextensionsv1.CustomResourceDefinition{}
-		if err := scheme.Convert(unversioned, crd, nil); err != nil {
-			return nil, fmt.Errorf("failed to convert crd from internal to v1: %s\nto v1: %s", c.String(), err)
+		v1crd := &apiextensionsv1.CustomResourceDefinition{}
+		if err := scheme.Convert(c, v1crd, nil); err != nil {
+			return nil, fmt.Errorf("failed to convert crd from v1beta1 to v1: %#v: %s", c, err)
 		}
-		return c, nil
+		return v1crd, nil
 	}
 	return nil, fmt.Errorf("unable to determine crd type")
 }

pkg/lib/crd/version.go

@@ -64,7 +64,7 @@ func NotEqual(currentCRD *apiextensionsv1.CustomResourceDefinition, oldCRD *apie
 			return true
 		}
 		equalValidation = reflect.DeepEqual(currentCRD.Spec.Versions[i].Schema, oldCRD.Spec.Versions[i].Schema)
-		if equalValidation == false {
+		if !equalValidation {
 			return true
 		}
 	}

test/e2e/installplan_e2e_test.go

@@ -488,9 +488,9 @@ var _ = Describe("Install Plan", func() {
 			newCRD          *apiextensions.CustomResourceDefinition
 		}
 
-		var min float64 = 2.1
-		var max float64 = 256.1
-		var newMax float64 = 50.1
+		var min float64 = 2
+		var max float64 = 256
+		var newMax float64 = 50
 		// generated outside of the test table so that the same naming can be used for both old and new CSVs
 		mainCRDPlural := "testcrd"
 
@@ -608,22 +608,22 @@ var _ = Describe("Install Plan", func() {
 						{
 							Name:    "v1alpha1",
 							Served:  true,
-							Storage: false,
-							Schema: &apiextensions.CustomResourceValidation{
-								OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
-									Type: "object",
+							Storage: true,
+						},
+					}
+					newCRD.Spec.Validation = &apiextensions.CustomResourceValidation{
+						OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+							Type: "object",
+							Properties: map[string]apiextensions.JSONSchemaProps{
+								"spec": {
+									Type:        "object",
+									Description: "Spec of a test object.",
 									Properties: map[string]apiextensions.JSONSchemaProps{
-										"spec": {
-											Type:        "object",
-											Description: "Spec of a test object.",
-											Properties: map[string]apiextensions.JSONSchemaProps{
-												"scalar": {
-													Type:        "number",
-													Description: "Scalar value that should have a min and max.",
-													Minimum:     &min,
-													Maximum:     &newMax,
-												},
-											},
+										"scalar": {
+											Type:        "number",
+											Description: "Scalar value that should have a min and max.",
+											Minimum:     &min,
+											Maximum:     &newMax,
 										},
 									},
 								},
@@ -807,6 +807,7 @@ var _ = Describe("Install Plan", func() {
 			fetchedInstallPlan, err := fetchInstallPlan(GinkgoT(), crc, installPlanName, completeOrFailedFunc)
 			require.NoError(GinkgoT(), err)
 			GinkgoT().Logf("Install plan %s fetched with status %s", fetchedInstallPlan.GetName(), fetchedInstallPlan.Status.Phase)
+			GinkgoT().Logf("install plan %#v", fetchedInstallPlan)
 			require.Equal(GinkgoT(), operatorsv1alpha1.InstallPlanPhaseComplete, fetchedInstallPlan.Status.Phase)
 
 			// Ensure that the desired resources have been created