diff --git a/docs/dev/clusteroperator.md b/docs/dev/clusteroperator.md index 5a067205bd..6e202a466c 100644 --- a/docs/dev/clusteroperator.md +++ b/docs/dev/clusteroperator.md @@ -20,6 +20,8 @@ When ClusterVersionOperator encounters a ClusterOperator Custom Resource, - the live instance `.status.conditions` report available, not progressing and not failed - It then continues to the next task. +ClusterVersionOperator will only deploy files with `.yaml`, `.yml`, or `.json` extensions, like `kubectl create -f DIR`. + **NOTE**: ClusterVersionOperator sweeps the manifests in the release payload in alphabetical order, therefore if the ClusterOperator Custom Resource exists before the deployment for the operator that is supposed to report the Custom Resource, ClusterVersionOperator will be stuck waiting and cannot proceed. ### What should be the contents of ClusterOperator Custom Resource in /manifests diff --git a/docs/dev/operators.md b/docs/dev/operators.md index ce6bc6601e..f1a4da7056 100644 --- a/docs/dev/operators.md +++ b/docs/dev/operators.md @@ -57,6 +57,8 @@ When your manifests are added to the release payload, they’ll be given a prefi 99_ingress-operator_02_deployment.yaml ``` +Only manifests with the extensions `.yaml`, `.yml`, or `.json` will be applied, like `kubectl create -f DIR`. + ### How do I get added as a special run level? Some operators need to run at a specific time in the release process (OLM, kube, openshift core operators, network, service CA). These components can ensure they run in a specific order across operators by prefixing their manifests with: @@ -77,6 +79,8 @@ Assigned runlevels - 10-19 - Kube operators (master team) - 20-29 - OpenShift core operators (master team) - 30-39 - OLM +- 50 - Machine API +- 51 - Machine Autoapprover ## How do I ensure the right images get used by my manifests? diff --git a/lib/manifest.go b/lib/manifest.go index fbc0b71341..e834335f60 100644 --- a/lib/manifest.go +++ b/lib/manifest.go @@ -21,7 +21,7 @@ type Manifest struct { Raw []byte GVK schema.GroupVersionKind - obj *unstructured.Unstructured + Obj *unstructured.Unstructured } // UnmarshalJSON unmarshals bytes of single kubernetes object to Manifest. @@ -51,12 +51,12 @@ func (m *Manifest) UnmarshalJSON(in []byte) error { } m.GVK = ud.GroupVersionKind() - m.obj = ud.DeepCopy() + m.Obj = ud.DeepCopy() return nil } // Object returns underlying metav1.Object -func (m *Manifest) Object() metav1.Object { return m.obj } +func (m *Manifest) Object() metav1.Object { return m.Obj } // ManifestsFromFiles reads files and returns Manifests in the same order. // files should be list of absolute paths for the manifests on disk. diff --git a/lib/manifest_test.go b/lib/manifest_test.go index 2822031754..f96aa37947 100644 --- a/lib/manifest_test.go +++ b/lib/manifest_test.go @@ -138,7 +138,7 @@ data: } for i := range got { - got[i].obj = nil + got[i].Obj = nil } if !reflect.DeepEqual(got, test.want) { @@ -277,7 +277,7 @@ data: } for i := range got { got[i].Raw = nil - got[i].obj = nil + got[i].Obj = nil } if !reflect.DeepEqual(got, test.want) { t.Fatalf("mismatch \ngot: %s \nwant: %s", spew.Sdump(got), spew.Sdump(test.want)) diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index 874ef87779..ce3261bd52 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -180,10 +180,10 @@ func New( glog.Warningf("The local payload is invalid - no current version can be determined from disk: %v", err) } else { // XXX: set this to the cincinnati version in preference - if _, err := semver.Parse(meta.imageRef.Name); err != nil { - glog.Warningf("The local payload name %q is not a valid semantic version - no current version will be reported: %v", meta.imageRef.Name, err) + if _, err := semver.Parse(meta.ImageRef.Name); err != nil { + glog.Warningf("The local payload name %q is not a valid semantic version - no current version will be reported: %v", meta.ImageRef.Name, err) } else { - optr.releaseVersion = meta.imageRef.Name + optr.releaseVersion = meta.ImageRef.Name } } @@ -314,16 +314,16 @@ func (optr *Operator) sync(key string) error { } update := configv1.Update{ - Version: payload.releaseVersion, - Payload: payload.releaseImage, + Version: payload.ReleaseVersion, + Payload: payload.ReleaseImage, } // if the current payload is already live, we are reconciling, not updating, // and we won't set the progressing status. - if availableAndUpdated && payload.manifestHash == original.Status.VersionHash { - glog.V(2).Infof("Reconciling cluster to version %s and image %s (hash=%s)", update.Version, update.Payload, payload.manifestHash) + if availableAndUpdated && payload.ManifestHash == original.Status.VersionHash { + glog.V(2).Infof("Reconciling cluster to version %s and image %s (hash=%s)", update.Version, update.Payload, payload.ManifestHash) } else { - glog.V(2).Infof("Updating the cluster to version %s and image %s (hash=%s)", update.Version, update.Payload, payload.manifestHash) + glog.V(2).Infof("Updating the cluster to version %s and image %s (hash=%s)", update.Version, update.Payload, payload.ManifestHash) if err := optr.syncProgressingStatus(original); err != nil { return err } @@ -340,7 +340,7 @@ func (optr *Operator) sync(key string) error { // update the status to indicate we have synced optr.setLastSyncAt(time.Now()) - return optr.syncAvailableStatus(original, update, payload.manifestHash) + return optr.syncAvailableStatus(original, update, payload.ManifestHash) } // availableUpdatesSync is triggered on cluster version change (and periodic requeues) to diff --git a/pkg/cvo/image.go b/pkg/cvo/image.go index aacbe3edea..e9277a0242 100644 --- a/pkg/cvo/image.go +++ b/pkg/cvo/image.go @@ -14,7 +14,7 @@ func ImageForShortName(name string) (string, error) { return "", errors.Wrapf(err, "error loading update payload from %q", defaultUpdatePayloadDir) } - for _, tag := range up.imageRef.Spec.Tags { + for _, tag := range up.ImageRef.Spec.Tags { if tag.Name == name { // we found the short name in ImageStream if tag.From != nil && tag.From.Kind == "DockerImage" { diff --git a/pkg/cvo/sync.go b/pkg/cvo/sync.go index c4696e5810..c652572601 100644 --- a/pkg/cvo/sync.go +++ b/pkg/cvo/sync.go @@ -50,19 +50,19 @@ func (optr *Operator) loadUpdatePayload(config *configv1.ClusterVersion) (*updat // syncUpdatePayload applies the manifests in the payload to the cluster. func (optr *Operator) syncUpdatePayload(config *configv1.ClusterVersion, payload *updatePayload) error { - version := payload.releaseVersion + version := payload.ReleaseVersion if len(version) == 0 { - version = payload.releaseImage + version = payload.ReleaseImage } - total := len(payload.manifests) + total := len(payload.Manifests) done := 0 var tasks []*syncTask - for i := range payload.manifests { + for i := range payload.Manifests { tasks = append(tasks, &syncTask{ index: i + 1, total: total, - manifest: &payload.manifests[i], + manifest: &payload.Manifests[i], backoff: optr.syncBackoff, }) } diff --git a/pkg/cvo/sync_test.go b/pkg/cvo/sync_test.go index 9daf5908d3..0d57828e1d 100644 --- a/pkg/cvo/sync_test.go +++ b/pkg/cvo/sync_test.go @@ -352,7 +352,7 @@ func TestSyncUpdatePayload(t *testing.T) { manifests = append(manifests, m) } - up := &updatePayload{releaseImage: "test", releaseVersion: "v0.0.0", manifests: manifests} + up := &updatePayload{ReleaseImage: "test", ReleaseVersion: "v0.0.0", Manifests: manifests} op := &Operator{} op.syncBackoff = wait.Backoff{Steps: 3} config := &configv1.ClusterVersion{} diff --git a/pkg/cvo/testdata/payloadtest/manifests/.gitkeep b/pkg/cvo/testdata/payloadtest/manifests/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pkg/cvo/testdata/payloadtest/release-manifests/file b/pkg/cvo/testdata/payloadtest/release-manifests/file new file mode 100644 index 0000000000..8212e8957c --- /dev/null +++ b/pkg/cvo/testdata/payloadtest/release-manifests/file @@ -0,0 +1,7 @@ +{ + "kind": "Test", + "apiVersion": "v1", + "metadata": { + "name": "file" + } +} \ No newline at end of file diff --git a/pkg/cvo/testdata/payloadtest/release-manifests/file.json b/pkg/cvo/testdata/payloadtest/release-manifests/file.json new file mode 100644 index 0000000000..0ccbf6b8b1 --- /dev/null +++ b/pkg/cvo/testdata/payloadtest/release-manifests/file.json @@ -0,0 +1,7 @@ +{ + "kind": "Test", + "apiVersion": "v1", + "metadata": { + "name": "file-json" + } +} \ No newline at end of file diff --git a/pkg/cvo/testdata/payloadtest/release-manifests/file.yaml b/pkg/cvo/testdata/payloadtest/release-manifests/file.yaml new file mode 100644 index 0000000000..55d3fa933f --- /dev/null +++ b/pkg/cvo/testdata/payloadtest/release-manifests/file.yaml @@ -0,0 +1,4 @@ +kind: Test +apiVersion: v1 +metadata: + name: file-yaml \ No newline at end of file diff --git a/pkg/cvo/testdata/payloadtest/release-manifests/file.yml b/pkg/cvo/testdata/payloadtest/release-manifests/file.yml new file mode 100644 index 0000000000..d6a790cfdf --- /dev/null +++ b/pkg/cvo/testdata/payloadtest/release-manifests/file.yml @@ -0,0 +1,4 @@ +kind: Test +apiVersion: v1 +metadata: + name: file-yml \ No newline at end of file diff --git a/pkg/cvo/testdata/payloadtest/release-manifests/image-references b/pkg/cvo/testdata/payloadtest/release-manifests/image-references new file mode 100644 index 0000000000..781a25f195 --- /dev/null +++ b/pkg/cvo/testdata/payloadtest/release-manifests/image-references @@ -0,0 +1,4 @@ +kind: ImageStream +apiVersion: image.openshift.io/v1 +metadata: + name: 1.0.0-abc \ No newline at end of file diff --git a/pkg/cvo/testdata/payloadtest/release-manifests/release-metadata b/pkg/cvo/testdata/payloadtest/release-manifests/release-metadata new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pkg/cvo/updatepayload.go b/pkg/cvo/updatepayload.go index 8106dd21e5..c28f464c80 100644 --- a/pkg/cvo/updatepayload.go +++ b/pkg/cvo/updatepayload.go @@ -20,22 +20,22 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/utils/pointer" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/cluster-version-operator/lib" "github.com/openshift/cluster-version-operator/lib/resourcebuilder" "github.com/openshift/cluster-version-operator/lib/resourceread" - configv1 "github.com/openshift/api/config/v1" ) type updatePayload struct { - releaseImage string - releaseVersion string + ReleaseImage string + ReleaseVersion string // XXX: cincinatti.json struct - imageRef *imagev1.ImageStream + ImageRef *imagev1.ImageStream // manifestHash is a hash of the manifests included in this payload - manifestHash string - manifests []lib.Manifest + ManifestHash string + Manifests []lib.Manifest } const ( @@ -45,7 +45,7 @@ const ( cvoManifestDir = "manifests" releaseManifestDir = "release-manifests" - cincinnatiJSONFile = "cincinnati.json" + cincinnatiJSONFile = "release-metadata" imageReferencesFile = "image-references" ) @@ -89,7 +89,7 @@ func loadUpdatePayloadMetadata(dir, releaseImage string) (*updatePayload, []payl preprocess: nil, skipFiles: sets.NewString(cjf, irf), }} - return &updatePayload{imageRef: imageRef, releaseImage: releaseImage, releaseVersion: imageRef.Name}, tasks, nil + return &updatePayload{ImageRef: imageRef, ReleaseImage: releaseImage, ReleaseVersion: imageRef.Name}, tasks, nil } func loadUpdatePayload(dir, releaseImage string) (*updatePayload, error) { @@ -111,6 +111,12 @@ func loadUpdatePayload(dir, releaseImage string) (*updatePayload, error) { continue } + switch filepath.Ext(file.Name()) { + case ".yaml", ".yml", ".json": + default: + continue + } + p := filepath.Join(task.idir, file.Name()) if task.skipFiles.Has(p) { continue @@ -150,8 +156,8 @@ func loadUpdatePayload(dir, releaseImage string) (*updatePayload, error) { hash.Write(manifest.Raw) } - payload.manifestHash = base64.URLEncoding.EncodeToString(hash.Sum(nil)) - payload.manifests = manifests + payload.ManifestHash = base64.URLEncoding.EncodeToString(hash.Sum(nil)) + payload.Manifests = manifests return payload, nil } diff --git a/pkg/cvo/updatepayload_test.go b/pkg/cvo/updatepayload_test.go new file mode 100644 index 0000000000..8839950ba3 --- /dev/null +++ b/pkg/cvo/updatepayload_test.go @@ -0,0 +1,121 @@ +package cvo + +import ( + "io/ioutil" + "path/filepath" + "reflect" + "testing" + + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + + imagev1 "github.com/openshift/api/image/v1" + "github.com/openshift/cluster-version-operator/lib" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/diff" +) + +func Test_loadUpdatePayload(t *testing.T) { + type args struct { + dir string + releaseImage string + } + tests := []struct { + name string + args args + want *updatePayload + wantErr bool + }{ + { + name: "ignore files without extensions, load metadata", + args: args{ + dir: filepath.Join("testdata", "payloadtest"), + releaseImage: "payload:1", + }, + want: &updatePayload{ + ReleaseImage: "payload:1", + ReleaseVersion: "1.0.0-abc", + ImageRef: &imagev1.ImageStream{ + TypeMeta: metav1.TypeMeta{ + Kind: "ImageStream", + APIVersion: "image.openshift.io/v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "1.0.0-abc", + }, + }, + ManifestHash: "6GC9TkkG9PA=", + Manifests: []lib.Manifest{ + { + Raw: mustRead(filepath.Join("testdata", "payloadtest", "release-manifests", "file.json")), + GVK: schema.GroupVersionKind{ + Kind: "Test", + Version: "v1", + }, + Obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Test", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "file-json", + }, + }, + }, + }, + { + Raw: []byte(`{"apiVersion":"v1","kind":"Test","metadata":{"name":"file-yaml"}}`), + GVK: schema.GroupVersionKind{ + Kind: "Test", + Version: "v1", + }, + Obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Test", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "file-yaml", + }, + }, + }, + }, + { + Raw: []byte(`{"apiVersion":"v1","kind":"Test","metadata":{"name":"file-yml"}}`), + GVK: schema.GroupVersionKind{ + Kind: "Test", + Version: "v1", + }, + Obj: &unstructured.Unstructured{ + Object: map[string]interface{}{ + "kind": "Test", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "file-yml", + }, + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := loadUpdatePayload(tt.args.dir, tt.args.releaseImage) + if (err != nil) != tt.wantErr { + t.Errorf("loadUpdatePayload() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("loadUpdatePayload() = %s", diff.ObjectReflectDiff(tt.want, got)) + } + }) + } +} + +func mustRead(path string) []byte { + data, err := ioutil.ReadFile(path) + if err != nil { + panic(err) + } + return data +}