diff --git a/api/v1/clusterextensionrevision_types.go b/api/v1/clusterextensionrevision_types.go index 69a116300..e048e1b54 100644 --- a/api/v1/clusterextensionrevision_types.go +++ b/api/v1/clusterextensionrevision_types.go @@ -24,15 +24,11 @@ import ( const ( ClusterExtensionRevisionKind = "ClusterExtensionRevision" - // ClusterExtensionRevisionTypeAvailable is the condition type that represents whether the - // ClusterExtensionRevision is available and has been successfully rolled out. + // Condition Types ClusterExtensionRevisionTypeAvailable = "Available" - - // ClusterExtensionRevisionTypeSucceeded is the condition type that represents whether the - // ClusterExtensionRevision rollout has succeeded. ClusterExtensionRevisionTypeSucceeded = "Succeeded" - // Condition reasons + // Condition Reasons ClusterExtensionRevisionReasonAvailable = "Available" ClusterExtensionRevisionReasonReconcileFailure = "ReconcileFailure" ClusterExtensionRevisionReasonRevisionValidationFailure = "RevisionValidationFailure" @@ -48,47 +44,22 @@ const ( // ClusterExtensionRevisionSpec defines the desired state of ClusterExtensionRevision. type ClusterExtensionRevisionSpec struct { - // lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. - // - // When set to "Active" (the default), the revision is actively managed and reconciled. - // When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. - // The revision is removed from the owner list of all objects previously under management. - // All objects that did not transition to a succeeding revision are deleted. - // - // Once a revision is set to "Archived", it cannot be un-archived. + // Specifies the lifecycle state of the ClusterExtensionRevision. // // +kubebuilder:default="Active" - // +kubebuilder:validation:Enum=Active;Archived - // +kubebuilder:validation:XValidation:rule="oldSelf == 'Active' || oldSelf == 'Archived' && oldSelf == self", message="cannot un-archive" + // +kubebuilder:validation:Enum=Active;Paused;Archived + // +kubebuilder:validation:XValidation:rule="oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' && oldSelf == self", message="can not un-archive" LifecycleState ClusterExtensionRevisionLifecycleState `json:"lifecycleState,omitempty"` - - // revision is a required, immutable sequence number representing a specific revision - // of the parent ClusterExtension. - // - // The revision field must be a positive integer. - // Each ClusterExtensionRevision belonging to the same parent ClusterExtension must have a unique revision number. - // The revision number must always be the previous revision number plus one, or 1 for the first revision. + // Revision is a sequence number representing a specific revision of the ClusterExtension instance. + // Must be positive. Each ClusterExtensionRevision of the same parent ClusterExtension needs to have + // a unique value assigned. It is immutable after creation. The new revision number must always be previous revision +1. // // +kubebuilder:validation:Required // +kubebuilder:validation:Minimum:=1 // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="revision is immutable" Revision int64 `json:"revision"` - - // phases is an optional, immutable list of phases that group objects to be applied together. - // - // Objects are organized into phases based on their Group-Kind. Common phases include: - // - namespaces: Namespace objects - // - policies: ResourceQuota, LimitRange, NetworkPolicy objects - // - rbac: ServiceAccount, Role, RoleBinding, ClusterRole, ClusterRoleBinding objects - // - crds: CustomResourceDefinition objects - // - storage: PersistentVolume, PersistentVolumeClaim, StorageClass objects - // - deploy: Deployment, StatefulSet, DaemonSet, Service, ConfigMap, Secret objects - // - publish: Ingress, APIService, Route, Webhook objects - // - // All objects in a phase are applied in no particular order. - // The revision progresses to the next phase only after all objects in the current phase pass their readiness probes. - // - // Once set, even if empty, the phases field is immutable. + // Phases are groups of objects that will be applied at the same time. + // All objects in the phase will have to pass their probes in order to progress to the next phase. // // +kubebuilder:validation:XValidation:rule="self == oldSelf || oldSelf.size() == 0", message="phases is immutable" // +listType=map @@ -104,62 +75,33 @@ const ( // ClusterExtensionRevisionLifecycleStateActive / "Active" is the default lifecycle state. ClusterExtensionRevisionLifecycleStateActive ClusterExtensionRevisionLifecycleState = "Active" // ClusterExtensionRevisionLifecycleStatePaused / "Paused" disables reconciliation of the ClusterExtensionRevision. - // Object changes will not be reconciled. However, status updates will be propagated. + // Only Status updates will still propagated, but object changes will not be reconciled. ClusterExtensionRevisionLifecycleStatePaused ClusterExtensionRevisionLifecycleState = "Paused" - // ClusterExtensionRevisionLifecycleStateArchived / "Archived" archives the revision for historical or auditing purposes. - // The revision is removed from the owner list of all other objects previously under management and all objects - // that did not transition to a succeeding revision are deleted. + // ClusterExtensionRevisionLifecycleStateArchived / "Archived" disables reconciliation while also "scaling to zero", + // which deletes all objects that are not excluded via the pausedFor property and + // removes itself from the owner list of all other objects previously under management. ClusterExtensionRevisionLifecycleStateArchived ClusterExtensionRevisionLifecycleState = "Archived" ) -// ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered -// complete only after all objects pass their status probes. +// ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time. +// All objects in the a phase will have to pass their probes in order to progress to the next phase. type ClusterExtensionRevisionPhase struct { - // name is a required identifier for this phase. - // - // phase names must follow the DNS label standard as defined in [RFC 1123]. - // They must contain only lowercase alphanumeric characters or hyphens (-), - // start and end with an alphanumeric character, and be no longer than 63 characters. - // - // Common phase names include: namespaces, policies, rbac, crds, storage, deploy, publish. - // - // [RFC 1123]: https://tools.ietf.org/html/rfc1123 + // Name identifies this phase. // // +kubebuilder:validation:MaxLength=63 // +kubebuilder:validation:Pattern=`^[a-z]([-a-z0-9]*[a-z0-9])?$` Name string `json:"name"` - - // objects is a required list of all Kubernetes objects that belong to this phase. - // - // All objects in this list are applied to the cluster in no particular order. + // Objects are a list of all the objects within this phase. Objects []ClusterExtensionRevisionObject `json:"objects"` } -// ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part -// of a phase, along with its collision protection settings. +// ClusterExtensionRevisionObject contains an object and settings for it. type ClusterExtensionRevisionObject struct { - // object is a required embedded Kubernetes object to be applied. - // - // This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. - // // +kubebuilder:validation:EmbeddedResource // +kubebuilder:pruning:PreserveUnknownFields Object unstructured.Unstructured `json:"object"` - - // collisionProtection controls whether the operator can adopt and modify objects - // that already exist on the cluster. - // - // When set to "Prevent" (the default), the operator only manages objects it created itself. - // This prevents ownership collisions. - // - // When set to "IfNoController", the operator can adopt and modify pre-existing objects - // that are not owned by another controller. - // This is useful for taking over management of manually-created resources. - // - // When set to "None", the operator can adopt and modify any pre-existing object, even if - // owned by another controller. - // Use this setting with extreme caution as it may cause multiple controllers to fight over - // the same resource, resulting in increased load on the API server and etcd. + // CollisionProtection controls whether OLM can adopt and modify objects + // already existing on the cluster or even owned by another controller. // // +kubebuilder:default="Prevent" // +kubebuilder:validation:Enum=Prevent;IfNoController;None @@ -186,27 +128,6 @@ const ( // ClusterExtensionRevisionStatus defines the observed state of a ClusterExtensionRevision. type ClusterExtensionRevisionStatus struct { - // conditions is an optional list of status conditions describing the state of the - // ClusterExtensionRevision. - // - // The Progressing condition represents whether the revision is actively rolling out: - // - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - // - When Progressing is not present, the revision is not currently in transition. - // - // The Available condition represents whether the revision has been successfully rolled out and is available: - // - When status is True and reason is Available, the revision has been successfully rolled out and all objects pass their readiness probes. - // - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. - // - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - // - When status is False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - // - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - // - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - // - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - // - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - // - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. - // - // The Succeeded condition represents whether the revision has successfully completed its rollout: - // - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. - // // +listType=map // +listMapKey=type // +optional @@ -216,24 +137,19 @@ type ClusterExtensionRevisionStatus struct { // +kubebuilder:object:root=true // +kubebuilder:resource:scope=Cluster // +kubebuilder:subresource:status + +// ClusterExtensionRevision is the Schema for the clusterextensionrevisions API // +kubebuilder:printcolumn:name="Available",type=string,JSONPath=`.status.conditions[?(@.type=='Available')].status` // +kubebuilder:printcolumn:name=Age,type=date,JSONPath=`.metadata.creationTimestamp` - -// ClusterExtensionRevision represents an immutable snapshot of Kubernetes objects -// for a specific version of a ClusterExtension. Each revision contains objects -// organized into phases that roll out sequentially. The same object can only be managed by a single revision -// at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded -// or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for -// posterity. type ClusterExtensionRevision struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata,omitempty"` - // spec defines the desired state of the ClusterExtensionRevision. + // spec is an optional field that defines the desired state of the ClusterExtension. // +optional Spec ClusterExtensionRevisionSpec `json:"spec,omitempty"` - // status is optional and defines the observed state of the ClusterExtensionRevision. + // status is an optional field that defines the observed state of the ClusterExtension. // +optional Status ClusterExtensionRevisionStatus `json:"status,omitempty"` } diff --git a/cmd/catalogd/main.go b/cmd/catalogd/main.go index af2463e2c..36f7b1675 100644 --- a/cmd/catalogd/main.go +++ b/cmd/catalogd/main.go @@ -59,7 +59,6 @@ import ( "github.com/operator-framework/operator-controller/internal/catalogd/storage" "github.com/operator-framework/operator-controller/internal/catalogd/webhook" sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers" - cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache" fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" httputil "github.com/operator-framework/operator-controller/internal/shared/util/http" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" @@ -255,8 +254,6 @@ func run(ctx context.Context) error { cacheOptions := crcache.Options{ ByObject: map[client.Object]crcache.ByObject{}, - // Memory optimization: strip managed fields and large annotations from cached objects - DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(), } saKey, err := sautil.GetServiceAccount() diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index c72ba60f2..5534244ac 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -78,7 +78,6 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render/registryv1" "github.com/operator-framework/operator-controller/internal/operator-controller/scheme" sharedcontrollers "github.com/operator-framework/operator-controller/internal/shared/controllers" - cacheutil "github.com/operator-framework/operator-controller/internal/shared/util/cache" fsutil "github.com/operator-framework/operator-controller/internal/shared/util/fs" httputil "github.com/operator-framework/operator-controller/internal/shared/util/http" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" @@ -258,8 +257,6 @@ func run() error { cfg.systemNamespace: {LabelSelector: k8slabels.Everything()}, }, DefaultLabelSelector: k8slabels.Nothing(), - // Memory optimization: strip managed fields and large annotations from cached objects - DefaultTransform: cacheutil.StripAnnotations(), } if features.OperatorControllerFeatureGate.Enabled(features.BoxcutterRuntime) { diff --git a/commitchecker.yaml b/commitchecker.yaml index 6bb0b5fac..0b1a24c57 100644 --- a/commitchecker.yaml +++ b/commitchecker.yaml @@ -1,4 +1,4 @@ -expectedMergeBase: 4e349e62c5314574f6194d64b1ff4508f2e9331f +expectedMergeBase: 045989d84a7570b1cfddeee47eae64d47245aff2 upstreamBranch: main upstreamOrg: operator-framework upstreamRepo: operator-controller diff --git a/hack/test/install-prometheus.sh b/hack/test/install-prometheus.sh index e2e13c96f..f458b2d01 100755 --- a/hack/test/install-prometheus.sh +++ b/hack/test/install-prometheus.sh @@ -38,11 +38,8 @@ echo "Patching namespace to ${PROMETHEUS_NAMESPACE}..." echo "Applying Prometheus base..." kubectl apply -k "$TMPDIR" --server-side -echo "Waiting for Prometheus Operator deployment to become available..." -kubectl wait --for=condition=Available deployment/prometheus-operator -n "$PROMETHEUS_NAMESPACE" --timeout=180s - echo "Waiting for Prometheus Operator pod to become ready..." -kubectl wait --for=condition=Ready pod -n "$PROMETHEUS_NAMESPACE" -l app.kubernetes.io/name=prometheus-operator --timeout=120s +kubectl wait --for=condition=Ready pod -n "$PROMETHEUS_NAMESPACE" -l app.kubernetes.io/name=prometheus-operator echo "Applying prometheus Helm chart..." ${HELM} template prometheus helm/prometheus ${PROMETHEUS_VALUES} | sed "s/cert-git-version/cert-${VERSION}/g" | kubectl apply -f - diff --git a/hack/tools/crd-generator/README.md b/hack/tools/crd-generator/README.md index 83fb63e21..433472167 100644 --- a/hack/tools/crd-generator/README.md +++ b/hack/tools/crd-generator/README.md @@ -33,14 +33,6 @@ A semi-colon separated list of enumerations, similar to the `+kubebuilder:valida An XValidation scheme, similar to the `+kubebuilder:validation:XValidation` scheme, but more limited. -* `Optional` - -Indicating that this field should not be listed as required in its parent. - -* `Required` - -Indicating that this field should be listed as required in its parent. - ## Experimental Description * Start Tag: `` @@ -52,18 +44,6 @@ All text between the tags is included in the experimental CRD, but removed from This is only useful if the field is included in the standard CRD, but there's additional meaning in the experimental CRD when feature gates are enabled. -## Standard Description - -* Start Tag: `` -* End Tag: `` - -Descriptive text that is only included as part of the field description within the standard CRD. -All text between the tags is included in the standard CRD, but removed from the experimental CRD. - -This is useful if the field is included in the standard CRD and has differing meaning than when the -field is used in the experimental CRD when feature gates are enabled. - - ## Exclude from CRD Description * Start Tag: `` diff --git a/hack/tools/crd-generator/main.go b/hack/tools/crd-generator/main.go index edc254494..9687489f4 100644 --- a/hack/tools/crd-generator/main.go +++ b/hack/tools/crd-generator/main.go @@ -23,7 +23,6 @@ import ( "log" "os" "regexp" - "slices" "strings" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -137,7 +136,7 @@ func runGenerator(args ...string) { if channel == StandardChannel && strings.Contains(version.Name, "alpha") { channelCrd.Spec.Versions[i].Served = false } - channelCrd.Spec.Versions[i].Schema.OpenAPIV3Schema.Properties = opconTweaksMap(channel, channelCrd.Spec.Versions[i].Schema.OpenAPIV3Schema) + version.Schema.OpenAPIV3Schema.Properties = opconTweaksMap(channel, version.Schema.OpenAPIV3Schema.Properties) } conv, err := crd.AsVersion(*channelCrd, apiextensionsv1.SchemeGroupVersion) @@ -180,51 +179,25 @@ func runGenerator(args ...string) { } } -// Apply Opcon specific tweaks to all properties in a map, and update the parent schema's required list according to opcon tags. -// For opcon validation optional/required tags, the parent schema's required list is mutated directly. -// TODO: if we need to support other conditions from opconTweaks, it will likely be preferable to convey the parent schema to facilitate direct alteration. -func opconTweaksMap(channel string, parentSchema *apiextensionsv1.JSONSchemaProps) map[string]apiextensionsv1.JSONSchemaProps { - props := parentSchema.Properties - +func opconTweaksMap(channel string, props map[string]apiextensionsv1.JSONSchemaProps) map[string]apiextensionsv1.JSONSchemaProps { for name := range props { jsonProps := props[name] - p, reqStatus := opconTweaks(channel, name, jsonProps) + p := opconTweaks(channel, name, jsonProps) if p == nil { delete(props, name) } else { props[name] = *p - // Update required list based on tag - switch reqStatus { - case statusRequired: - if !slices.Contains(parentSchema.Required, name) { - parentSchema.Required = append(parentSchema.Required, name) - } - case statusOptional: - parentSchema.Required = slices.DeleteFunc(parentSchema.Required, func(s string) bool { return s == name }) - default: - // "" (unspecified) means keep existing status - } } } return props } -const ( - statusRequired = "required" - statusOptional = "optional" - statusNoOpinion = "" -) - // Custom Opcon API Tweaks for tags prefixed with `") { - return nil, statusNoOpinion + return nil } } @@ -246,7 +219,7 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche numValid++ jsonProps.Enum = []apiextensionsv1.JSON{} - for val := range strings.SplitSeq(enumMatch[1], ";") { + for _, val := range strings.Split(enumMatch[1], ";") { jsonProps.Enum = append(jsonProps.Enum, apiextensionsv1.JSON{Raw: []byte("\"" + val + "\"")}) } } @@ -264,28 +237,6 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche Rule: celMatch[2], }) } - optReqRe := regexp.MustCompile(validationPrefix + "(Optional|Required)>") - optReqMatches := optReqRe.FindAllStringSubmatch(jsonProps.Description, 64) - hasOptional := false - hasRequired := false - for _, optReqMatch := range optReqMatches { - if len(optReqMatch) != 2 { - log.Fatalf("Invalid %s Optional/Required tag for %s", validationPrefix, name) - } - - numValid++ - switch optReqMatch[1] { - case "Optional": - hasOptional = true - requiredStatus = statusOptional - case "Required": - hasRequired = true - requiredStatus = statusRequired - } - } - if hasOptional && hasRequired { - log.Fatalf("Field %s has both Optional and Required validation tags for channel %s", name, channel) - } } if numValid < numExpressions { @@ -295,43 +246,34 @@ func opconTweaks(channel string, name string, jsonProps apiextensionsv1.JSONSche jsonProps.Description = formatDescription(jsonProps.Description, channel, name) if len(jsonProps.Properties) > 0 { - jsonProps.Properties = opconTweaksMap(channel, &jsonProps) + jsonProps.Properties = opconTweaksMap(channel, jsonProps.Properties) } else if jsonProps.Items != nil && jsonProps.Items.Schema != nil { - jsonProps.Items.Schema, _ = opconTweaks(channel, name, *jsonProps.Items.Schema) + jsonProps.Items.Schema = opconTweaks(channel, name, *jsonProps.Items.Schema) } - return &jsonProps, requiredStatus + return &jsonProps } func formatDescription(description string, channel string, name string) string { - tagset := []struct { - channel string - tag string - }{ - {channel: ExperimentalChannel, tag: "opcon:standard:description"}, - {channel: StandardChannel, tag: "opcon:experimental:description"}, - } - for _, ts := range tagset { - startTag := fmt.Sprintf("<%s>", ts.tag) - endTag := fmt.Sprintf("", ts.tag) - if channel == ts.channel && strings.Contains(description, ts.tag) { - regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*` - re := regexp.MustCompile(regexPattern) - match := re.FindStringSubmatch(description) - if len(match) != 2 { - log.Fatalf("Invalid %s tag for %s", startTag, name) - } - description = re.ReplaceAllString(description, "\n\n") - } else { - description = strings.ReplaceAll(description, startTag, "") - description = strings.ReplaceAll(description, endTag, "") + startTag := "" + endTag := "" + if channel == StandardChannel && strings.Contains(description, startTag) { + regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*` + re := regexp.MustCompile(regexPattern) + match := re.FindStringSubmatch(description) + if len(match) != 2 { + log.Fatalf("Invalid tag for %s", name) } + description = re.ReplaceAllString(description, "\n\n") + } else { + description = strings.ReplaceAll(description, startTag, "") + description = strings.ReplaceAll(description, endTag, "") } // Comments within "opcon:util:excludeFromCRD" tag are not included in the generated CRD and all trailing \n operators before // and after the tags are removed and replaced with three \n operators. - startTag := "" - endTag := "" + startTag = "" + endTag = "" if strings.Contains(description, startTag) { regexPattern := `\n*` + regexp.QuoteMeta(startTag) + `(?s:(.*?))` + regexp.QuoteMeta(endTag) + `\n*` re := regexp.MustCompile(regexPattern) diff --git a/hack/tools/crd-generator/main_test.go b/hack/tools/crd-generator/main_test.go index 99f71a497..d2eb28d61 100644 --- a/hack/tools/crd-generator/main_test.go +++ b/hack/tools/crd-generator/main_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/require" - apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" ) const controllerToolsVersion = "v0.19.0" @@ -76,252 +75,6 @@ func TestTags(t *testing.T) { compareFiles(t, f1, f2) } -func TestFormatDescription(t *testing.T) { - tests := []struct { - name string - channel string - fieldName string - input string - expected string - }{ - { - name: "standard channel removes experimental description", - channel: StandardChannel, - fieldName: "testField", - input: "Base description.\n\nExperimental content.\n\nMore content.", - expected: "Base description.\n\nMore content.", - }, - { - name: "experimental channel removes standard description", - channel: ExperimentalChannel, - fieldName: "testField", - input: "Base description.\n\nStandard content.\n\nMore content.", - expected: "Base description.\n\nMore content.", - }, - { - name: "excludeFromCRD tag removes content", - channel: StandardChannel, - fieldName: "testField", - input: "Before.\n\n\nExcluded content.\n\n\nAfter.", - expected: "Before.\n\nAfter.", - }, - { - name: "three hyphens removes trailing content", - channel: StandardChannel, - fieldName: "testField", - input: "Visible content.\n---\nHidden content after separator.", - expected: "Visible content.", - }, - { - name: "multiple newlines collapsed to double", - channel: StandardChannel, - fieldName: "testField", - input: "Line one.\n\n\n\n\nLine two.", - expected: "Line one.\n\nLine two.", - }, - { - name: "trailing newlines removed", - channel: StandardChannel, - fieldName: "testField", - input: "Content with trailing newlines.\n\n\n", - expected: "Content with trailing newlines.", - }, - { - name: "combined tags and formatting", - channel: ExperimentalChannel, - fieldName: "testField", - input: "Main text.\n\nStandard only.\n\n\n\n\nInternal notes.\n\n\nFinal text.\n\n\n", - expected: "Main text.\n\nFinal text.", - }, - { - name: "empty input", - channel: StandardChannel, - fieldName: "testField", - input: "", - expected: "", - }, - { - name: "no tags plain text", - channel: StandardChannel, - fieldName: "testField", - input: "Simple description without any tags.", - expected: "Simple description without any tags.", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := formatDescription(tt.input, tt.channel, tt.fieldName) - require.Equal(t, tt.expected, result) - }) - } -} - -// TestOpconTweaksOptionalRequired tests the opconTweaks function for handling -// optional and required tags in field descriptions. -func TestOpconTweaksOptionalRequired(t *testing.T) { - tests := []struct { - name string - channel string - fieldName string - description string - expectedStatus string - }{ - { - name: "optional tag in standard channel", - channel: StandardChannel, - fieldName: "testField", - description: "Field description.\n", - expectedStatus: statusOptional, - }, - { - name: "required tag in standard channel", - channel: StandardChannel, - fieldName: "testField", - description: "Field description.\n", - expectedStatus: statusRequired, - }, - { - name: "optional tag in experimental channel", - channel: ExperimentalChannel, - fieldName: "testField", - description: "Field description.\n", - expectedStatus: statusOptional, - }, - { - name: "required tag in experimental channel", - channel: ExperimentalChannel, - fieldName: "testField", - description: "Field description.\n", - expectedStatus: statusRequired, - }, - { - name: "no validation tag", - channel: StandardChannel, - fieldName: "testField", - description: "Field description without tags.", - expectedStatus: statusNoOpinion, - }, - { - name: "experimental tag in standard channel ignored", - channel: StandardChannel, - fieldName: "testField", - description: "Field description.\n", - expectedStatus: statusNoOpinion, - }, - { - name: "standard tag in experimental channel ignored", - channel: ExperimentalChannel, - fieldName: "testField", - description: "Field description.\n", - expectedStatus: statusNoOpinion, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - jsonProps := apiextensionsv1.JSONSchemaProps{ - Description: tt.description, - Type: "string", - } - _, status := opconTweaks(tt.channel, tt.fieldName, jsonProps) - require.Equal(t, tt.expectedStatus, status) - }) - } -} - -// TestOpconTweaksMapRequiredList tests the opconTweaksMap function for correctly -// updating the required list based on field descriptions. -func TestOpconTweaksMapRequiredList(t *testing.T) { - tests := []struct { - name string - channel string - props map[string]apiextensionsv1.JSONSchemaProps - existingRequired []string - expectedRequired []string - }{ - { - name: "add field to required list if not required but opcon required", - channel: StandardChannel, - props: map[string]apiextensionsv1.JSONSchemaProps{ - "field1": { - Description: "Field 1.\n", - Type: "string", - }, - }, - existingRequired: []string{}, - expectedRequired: []string{"field1"}, - }, - { - name: "remove field from required list if required but opcon optional", - channel: StandardChannel, - props: map[string]apiextensionsv1.JSONSchemaProps{ - "field1": { - Description: "Field 1.\n", - Type: "string", - }, - }, - existingRequired: []string{"field1"}, - expectedRequired: []string{}, - }, - { - name: "preserve existing required without overriding opcon tag", - channel: StandardChannel, - props: map[string]apiextensionsv1.JSONSchemaProps{ - "field1": { - Description: "Field 1 without tag.", - Type: "string", - }, - }, - existingRequired: []string{"field1"}, - expectedRequired: []string{"field1"}, - }, - { - name: "multiple fields with mixed optional/required tags", - channel: StandardChannel, - props: map[string]apiextensionsv1.JSONSchemaProps{ - "field1": { - Description: "Field 1.\n", - Type: "string", - }, - "field2": { - Description: "Field 2.\n", - Type: "string", - }, - "field3": { - Description: "Field 3 without tag.", - Type: "string", - }, - }, - existingRequired: []string{"field2", "field3"}, - expectedRequired: []string{"field3", "field1"}, - }, - { - name: "no duplicate in required list when tag/opcon-tag both required", - channel: StandardChannel, - props: map[string]apiextensionsv1.JSONSchemaProps{ - "field1": { - Description: "Field 1.\n", - Type: "string", - }, - }, - existingRequired: []string{"field1"}, - expectedRequired: []string{"field1"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - parentSchema := &apiextensionsv1.JSONSchemaProps{ - Properties: tt.props, - Required: tt.existingRequired, - } - opconTweaksMap(tt.channel, parentSchema) - require.ElementsMatch(t, tt.expectedRequired, parentSchema.Required) - }) - } -} - func compareFiles(t *testing.T, file1, file2 string) { f1, err := os.Open(file1) require.NoError(t, err) diff --git a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml index 1a3a8b021..b25e57903 100644 --- a/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml +++ b/helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensionrevisions.yaml @@ -25,13 +25,8 @@ spec: name: v1 schema: openAPIV3Schema: - description: |- - ClusterExtensionRevision represents an immutable snapshot of Kubernetes objects - for a specific version of a ClusterExtension. Each revision contains objects - organized into phases that roll out sequentially. The same object can only be managed by a single revision - at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded - or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for - posterity. + description: ClusterExtensionRevision is the Schema for the clusterextensionrevisions + API properties: apiVersion: description: |- @@ -51,100 +46,53 @@ spec: metadata: type: object spec: - description: spec defines the desired state of the ClusterExtensionRevision. + description: spec is an optional field that defines the desired state + of the ClusterExtension. properties: lifecycleState: default: Active - description: |- - lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. - - When set to "Active" (the default), the revision is actively managed and reconciled. - When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. - The revision is removed from the owner list of all objects previously under management. - All objects that did not transition to a succeeding revision are deleted. - - Once a revision is set to "Archived", it cannot be un-archived. + description: Specifies the lifecycle state of the ClusterExtensionRevision. enum: - Active + - Paused - Archived type: string x-kubernetes-validations: - - message: cannot un-archive - rule: oldSelf == 'Active' || oldSelf == 'Archived' && oldSelf == - self + - message: can not un-archive + rule: oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' + && oldSelf == self phases: description: |- - phases is an optional, immutable list of phases that group objects to be applied together. - - Objects are organized into phases based on their Group-Kind. Common phases include: - - namespaces: Namespace objects - - policies: ResourceQuota, LimitRange, NetworkPolicy objects - - rbac: ServiceAccount, Role, RoleBinding, ClusterRole, ClusterRoleBinding objects - - crds: CustomResourceDefinition objects - - storage: PersistentVolume, PersistentVolumeClaim, StorageClass objects - - deploy: Deployment, StatefulSet, DaemonSet, Service, ConfigMap, Secret objects - - publish: Ingress, APIService, Route, Webhook objects - - All objects in a phase are applied in no particular order. - The revision progresses to the next phase only after all objects in the current phase pass their readiness probes. - - Once set, even if empty, the phases field is immutable. + Phases are groups of objects that will be applied at the same time. + All objects in the phase will have to pass their probes in order to progress to the next phase. items: description: |- - ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered - complete only after all objects pass their status probes. + ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time. + All objects in the a phase will have to pass their probes in order to progress to the next phase. properties: name: - description: |- - name is a required identifier for this phase. - - phase names must follow the DNS label standard as defined in [RFC 1123]. - They must contain only lowercase alphanumeric characters or hyphens (-), - start and end with an alphanumeric character, and be no longer than 63 characters. - - Common phase names include: namespaces, policies, rbac, crds, storage, deploy, publish. - - [RFC 1123]: https://tools.ietf.org/html/rfc1123 + description: Name identifies this phase. maxLength: 63 pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ type: string objects: - description: |- - objects is a required list of all Kubernetes objects that belong to this phase. - - All objects in this list are applied to the cluster in no particular order. + description: Objects are a list of all the objects within this + phase. items: - description: |- - ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part - of a phase, along with its collision protection settings. + description: ClusterExtensionRevisionObject contains an object + and settings for it. properties: collisionProtection: default: Prevent description: |- - collisionProtection controls whether the operator can adopt and modify objects - that already exist on the cluster. - - When set to "Prevent" (the default), the operator only manages objects it created itself. - This prevents ownership collisions. - - When set to "IfNoController", the operator can adopt and modify pre-existing objects - that are not owned by another controller. - This is useful for taking over management of manually-created resources. - - When set to "None", the operator can adopt and modify any pre-existing object, even if - owned by another controller. - Use this setting with extreme caution as it may cause multiple controllers to fight over - the same resource, resulting in increased load on the API server and etcd. + CollisionProtection controls whether OLM can adopt and modify objects + already existing on the cluster or even owned by another controller. enum: - Prevent - IfNoController - None type: string object: - description: |- - object is a required embedded Kubernetes object to be applied. - - This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true @@ -165,12 +113,9 @@ spec: rule: self == oldSelf || oldSelf.size() == 0 revision: description: |- - revision is a required, immutable sequence number representing a specific revision - of the parent ClusterExtension. - - The revision field must be a positive integer. - Each ClusterExtensionRevision belonging to the same parent ClusterExtension must have a unique revision number. - The revision number must always be the previous revision number plus one, or 1 for the first revision. + Revision is a sequence number representing a specific revision of the ClusterExtension instance. + Must be positive. Each ClusterExtensionRevision of the same parent ClusterExtension needs to have + a unique value assigned. It is immutable after creation. The new revision number must always be previous revision +1. format: int64 minimum: 1 type: integer @@ -181,31 +126,10 @@ spec: - revision type: object status: - description: status is optional and defines the observed state of the - ClusterExtensionRevision. + description: status is an optional field that defines the observed state + of the ClusterExtension. properties: conditions: - description: |- - conditions is an optional list of status conditions describing the state of the - ClusterExtensionRevision. - - The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - - When Progressing is not present, the revision is not currently in transition. - - The Available condition represents whether the revision has been successfully rolled out and is available: - - When status is True and reason is Available, the revision has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. - - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - - When status is False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. - - The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/helm/olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml b/helm/olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml deleted file mode 100644 index 7e6ec2e33..000000000 --- a/helm/olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml +++ /dev/null @@ -1,22 +0,0 @@ -{{- if and .Values.options.catalogd.enabled .Values.options.catalogd.podDisruptionBudget.enabled }} -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: catalogd-controller-manager - namespace: {{ .Values.namespaces.olmv1.name }} - labels: - app.kubernetes.io/name: catalogd - {{- include "olmv1.labels" . | nindent 4 }} - annotations: - {{- include "olmv1.annotations" . | nindent 4 }} -spec: - {{- if .Values.options.catalogd.podDisruptionBudget.minAvailable }} - minAvailable: {{ .Values.options.catalogd.podDisruptionBudget.minAvailable }} - {{- end }} - {{- if .Values.options.catalogd.podDisruptionBudget.maxUnavailable }} - maxUnavailable: {{ .Values.options.catalogd.podDisruptionBudget.maxUnavailable }} - {{- end }} - selector: - matchLabels: - control-plane: catalogd-controller-manager -{{- end }} diff --git a/helm/olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml b/helm/olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml deleted file mode 100644 index 0d98d528f..000000000 --- a/helm/olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml +++ /dev/null @@ -1,22 +0,0 @@ -{{- if and .Values.options.operatorController.enabled .Values.options.operatorController.podDisruptionBudget.enabled }} -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: operator-controller-controller-manager - namespace: {{ .Values.namespaces.olmv1.name }} - labels: - app.kubernetes.io/name: operator-controller - {{- include "olmv1.labels" . | nindent 4 }} - annotations: - {{- include "olmv1.annotations" . | nindent 4 }} -spec: - {{- if .Values.options.operatorController.podDisruptionBudget.minAvailable }} - minAvailable: {{ .Values.options.operatorController.podDisruptionBudget.minAvailable }} - {{- end }} - {{- if .Values.options.operatorController.podDisruptionBudget.maxUnavailable }} - maxUnavailable: {{ .Values.options.operatorController.podDisruptionBudget.maxUnavailable }} - {{- end }} - selector: - matchLabels: - control-plane: operator-controller-controller-manager -{{- end }} diff --git a/helm/olmv1/values.yaml b/helm/olmv1/values.yaml index cb454f625..5ab9d7672 100644 --- a/helm/olmv1/values.yaml +++ b/helm/olmv1/values.yaml @@ -12,9 +12,6 @@ options: features: enabled: [] disabled: [] - podDisruptionBudget: - enabled: true - minAvailable: 1 catalogd: enabled: true deployment: @@ -23,9 +20,6 @@ options: features: enabled: [] disabled: [] - podDisruptionBudget: - enabled: true - minAvailable: 1 certManager: enabled: false e2e: diff --git a/internal/catalogd/controllers/core/clustercatalog_controller.go b/internal/catalogd/controllers/core/clustercatalog_controller.go index 3d7fd935c..e968db7b9 100644 --- a/internal/catalogd/controllers/core/clustercatalog_controller.go +++ b/internal/catalogd/controllers/core/clustercatalog_controller.go @@ -41,7 +41,6 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/catalogd/storage" imageutil "github.com/operator-framework/operator-controller/internal/shared/util/image" - k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s" ) const ( @@ -108,7 +107,7 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Do checks before any Update()s, as Update() may modify the resource structure! updateStatus := !equality.Semantic.DeepEqual(existingCatsrc.Status, reconciledCatsrc.Status) updateFinalizers := !equality.Semantic.DeepEqual(existingCatsrc.Finalizers, reconciledCatsrc.Finalizers) - unexpectedFieldsChanged := k8sutil.CheckForUnexpectedFieldChange(&existingCatsrc, reconciledCatsrc) + unexpectedFieldsChanged := checkForUnexpectedFieldChange(existingCatsrc, *reconciledCatsrc) if unexpectedFieldsChanged { panic("spec or metadata changed by reconciler") @@ -116,8 +115,8 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque // Save the finalizers off to the side. If we update the status, the reconciledCatsrc will be updated // to contain the new state of the ClusterCatalog, which contains the status update, but (critically) - // does not contain the finalizers. After the status update, we will use the saved finalizers in the - // CreateOrPatch() + // does not contain the finalizers. After the status update, we need to re-add the finalizers to the + // reconciledCatsrc before updating the object. finalizers := reconciledCatsrc.Finalizers if updateStatus { @@ -126,12 +125,10 @@ func (r *ClusterCatalogReconciler) Reconcile(ctx context.Context, req ctrl.Reque } } + reconciledCatsrc.Finalizers = finalizers + if updateFinalizers { - // Use CreateOrPatch to update finalizers on the server - if _, err := controllerutil.CreateOrPatch(ctx, r.Client, reconciledCatsrc, func() error { - reconciledCatsrc.Finalizers = finalizers - return nil - }); err != nil { + if err := r.Update(ctx, reconciledCatsrc); err != nil { reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating finalizers: %v", err)) } } @@ -418,6 +415,13 @@ func (r *ClusterCatalogReconciler) needsPoll(lastSuccessfulPoll time.Time, catal return nextPoll.Before(time.Now()) } +// Compare resources - ignoring status & metadata.finalizers +func checkForUnexpectedFieldChange(a, b ocv1.ClusterCatalog) bool { + a.Status, b.Status = ocv1.ClusterCatalogStatus{}, ocv1.ClusterCatalogStatus{} + a.Finalizers, b.Finalizers = []string{}, []string{} + return !equality.Semantic.DeepEqual(a, b) +} + type finalizerFunc func(ctx context.Context, obj client.Object) (crfinalizer.Result, error) func (f finalizerFunc) Finalize(ctx context.Context, obj client.Object) (crfinalizer.Result, error) { diff --git a/internal/operator-controller/applier/boxcutter.go b/internal/operator-controller/applier/boxcutter.go index 3895b49df..6abcd0c43 100644 --- a/internal/operator-controller/applier/boxcutter.go +++ b/internal/operator-controller/applier/boxcutter.go @@ -27,7 +27,6 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" - "github.com/operator-framework/operator-controller/internal/shared/util/cache" ) const ( @@ -67,9 +66,6 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease( maps.Copy(labels, objectLabels) obj.SetLabels(labels) - // Memory optimization: strip large annotations - // Note: ApplyStripTransform never returns an error in practice - _ = cache.ApplyStripAnnotationsTransform(&obj) sanitizedUnstructured(ctx, &obj) objs = append(objs, ocv1.ClusterExtensionRevisionObject{ @@ -121,10 +117,6 @@ func (r *SimpleRevisionGenerator) GenerateRevision( unstr := unstructured.Unstructured{Object: unstrObj} unstr.SetGroupVersionKind(gvk) - // Memory optimization: strip large annotations - if err := cache.ApplyStripAnnotationsTransform(&unstr); err != nil { - return nil, err - } sanitizedUnstructured(ctx, &unstr) objs = append(objs, ocv1.ClusterExtensionRevisionObject{ diff --git a/internal/operator-controller/controllers/clusterextension_controller.go b/internal/operator-controller/controllers/clusterextension_controller.go index 2b2f0d532..ef8cbd5f6 100644 --- a/internal/operator-controller/controllers/clusterextension_controller.go +++ b/internal/operator-controller/controllers/clusterextension_controller.go @@ -35,7 +35,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" crcontroller "sigs.k8s.io/controller-runtime/pkg/controller" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/event" crhandler "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/log" @@ -49,7 +48,6 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" - k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s" ) const ( @@ -137,28 +135,25 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req updateFinalizers := !equality.Semantic.DeepEqual(existingExt.Finalizers, reconciledExt.Finalizers) // If any unexpected fields have changed, panic before updating the resource - unexpectedFieldsChanged := k8sutil.CheckForUnexpectedFieldChange(existingExt, reconciledExt) + unexpectedFieldsChanged := checkForUnexpectedClusterExtensionFieldChange(*existingExt, *reconciledExt) if unexpectedFieldsChanged { panic("spec or metadata changed by reconciler") } // Save the finalizers off to the side. If we update the status, the reconciledExt will be updated // to contain the new state of the ClusterExtension, which contains the status update, but (critically) - // does not contain the finalizers. After the status update, we will use the saved finalizers in the - // CreateOrPatch() + // does not contain the finalizers. After the status update, we need to re-add the finalizers to the + // reconciledExt before updating the object. finalizers := reconciledExt.Finalizers if updateStatus { if err := r.Client.Status().Update(ctx, reconciledExt); err != nil { reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating status: %v", err)) } } + reconciledExt.Finalizers = finalizers if updateFinalizers { - // Use CreateOrPatch to update finalizers on the server - if _, err := controllerutil.CreateOrPatch(ctx, r.Client, reconciledExt, func() error { - reconciledExt.Finalizers = finalizers - return nil - }); err != nil { + if err := r.Update(ctx, reconciledExt); err != nil { reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating finalizers: %v", err)) } } @@ -184,6 +179,13 @@ func ensureAllConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.C } } +// Compare resources - ignoring status & metadata.finalizers +func checkForUnexpectedClusterExtensionFieldChange(a, b ocv1.ClusterExtension) bool { + a.Status, b.Status = ocv1.ClusterExtensionStatus{}, ocv1.ClusterExtensionStatus{} + a.Finalizers, b.Finalizers = []string{}, []string{} + return !equality.Semantic.DeepEqual(a, b) +} + // SetDeprecationStatus will set the appropriate deprecation statuses for a ClusterExtension // based on the provided bundle func SetDeprecationStatus(ext *ocv1.ClusterExtension, bundleName string, deprecation *declcfg.Deprecation) { diff --git a/internal/shared/util/cache/transform.go b/internal/shared/util/cache/transform.go deleted file mode 100644 index 50a553039..000000000 --- a/internal/shared/util/cache/transform.go +++ /dev/null @@ -1,91 +0,0 @@ -package cache - -import ( - "maps" - - toolscache "k8s.io/client-go/tools/cache" - crcache "sigs.k8s.io/controller-runtime/pkg/cache" - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// stripAnnotations removes memory-heavy annotations that aren't needed for controller operations. -func stripAnnotations(obj interface{}) (interface{}, error) { - if metaObj, ok := obj.(client.Object); ok { - // Remove the last-applied-configuration annotation which can be very large - // Clone the annotations map to avoid modifying shared references - annotations := metaObj.GetAnnotations() - if annotations != nil { - annotations = maps.Clone(annotations) - delete(annotations, "kubectl.kubernetes.io/last-applied-configuration") - if len(annotations) == 0 { - metaObj.SetAnnotations(nil) - } else { - metaObj.SetAnnotations(annotations) - } - } - } - return obj, nil -} - -// StripManagedFieldsAndAnnotations returns a cache transform function that removes -// memory-heavy fields that aren't needed for controller operations. -// This significantly reduces memory usage in informer caches by removing: -// - Managed fields (can be several KB per object) -// - kubectl.kubernetes.io/last-applied-configuration annotation (can be very large) -// -// Use this function as a DefaultTransform in controller-runtime cache.Options -// to reduce memory overhead across all cached objects. -// -// Example: -// -// cacheOptions := cache.Options{ -// DefaultTransform: cacheutil.StripManagedFieldsAndAnnotations(), -// } -func StripManagedFieldsAndAnnotations() toolscache.TransformFunc { - // Use controller-runtime's built-in TransformStripManagedFields and compose it - // with our custom annotation stripping transform - managedFieldsTransform := crcache.TransformStripManagedFields() - - return func(obj interface{}) (interface{}, error) { - // First strip managed fields using controller-runtime's transform - obj, err := managedFieldsTransform(obj) - if err != nil { - return obj, err - } - - // Then strip the large annotations - return stripAnnotations(obj) - } -} - -// StripAnnotations returns a cache transform function that removes -// memory-heavy annotation fields that aren't needed for controller operations. -// This significantly reduces memory usage in informer caches by removing: -// - kubectl.kubernetes.io/last-applied-configuration annotation (can be very large) -// -// Use this function as a DefaultTransform in controller-runtime cache.Options -// to reduce memory overhead across all cached objects. -// -// Example: -// -// cacheOptions := cache.Options{ -// DefaultTransform: cacheutil.StripAnnotations(), -// } -func StripAnnotations() toolscache.TransformFunc { - return func(obj interface{}) (interface{}, error) { - // Strip the large annotations - return stripAnnotations(obj) - } -} - -// ApplyStripAnnotationsTransform applies the strip transform directly to an object. -// This is a convenience function for cases where you need to strip fields -// from an object outside of the cache transform context. -// -// Note: This function never returns an error in practice, but returns error -// to satisfy the TransformFunc interface. -func ApplyStripAnnotationsTransform(obj client.Object) error { - transform := StripAnnotations() - _, err := transform(obj) - return err -} diff --git a/internal/shared/util/k8s/k8s.go b/internal/shared/util/k8s/k8s.go deleted file mode 100644 index a8a51a78a..000000000 --- a/internal/shared/util/k8s/k8s.go +++ /dev/null @@ -1,54 +0,0 @@ -package k8s - -import ( - "reflect" - - "k8s.io/apimachinery/pkg/api/equality" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - -// CheckForUnexpectedFieldChange compares two Kubernetes objects and returns true -// if their annotations, labels, or spec have changed. This is useful for detecting -// unexpected modifications during reconciliation. -// -// The function compares: -// - Annotations (via GetAnnotations) -// - Labels (via GetLabels) -// - Spec (using reflection to access the Spec field, with semantic equality) -// -// Status and finalizers are intentionally not compared, as these are expected -// to change during reconciliation. -// -// This function uses reflection to access the Spec field, so no explicit GetSpec() -// method is required. The objects must have a field named "Spec". -func CheckForUnexpectedFieldChange(a, b metav1.Object) bool { - if !equality.Semantic.DeepEqual(a.GetAnnotations(), b.GetAnnotations()) { - return true - } - if !equality.Semantic.DeepEqual(a.GetLabels(), b.GetLabels()) { - return true - } - - // Use reflection to access the Spec field - aVal := reflect.ValueOf(a) - bVal := reflect.ValueOf(b) - - // Handle pointer types - if aVal.Kind() == reflect.Ptr { - aVal = aVal.Elem() - } - if bVal.Kind() == reflect.Ptr { - bVal = bVal.Elem() - } - - // Get the Spec field from both objects - aSpec := aVal.FieldByName("Spec") - bSpec := bVal.FieldByName("Spec") - - // If either Spec field is invalid, return false (no change detected) - if !aSpec.IsValid() || !bSpec.IsValid() { - return false - } - - return !equality.Semantic.DeepEqual(aSpec.Interface(), bSpec.Interface()) -} diff --git a/internal/shared/util/k8s/k8s_test.go b/internal/shared/util/k8s/k8s_test.go deleted file mode 100644 index b4f599633..000000000 --- a/internal/shared/util/k8s/k8s_test.go +++ /dev/null @@ -1,227 +0,0 @@ -package k8s - -import ( - "testing" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - ocv1 "github.com/operator-framework/operator-controller/api/v1" -) - -func TestCheckForUnexpectedFieldChange(t *testing.T) { - tests := []struct { - name string - a ocv1.ClusterExtension - b ocv1.ClusterExtension - expected bool - }{ - { - name: "no changes", - a: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - Finalizers: []string{"finalizer1"}, - }, - Spec: ocv1.ClusterExtensionSpec{ - Source: ocv1.SourceConfig{ - SourceType: "Catalog", - }, - }, - Status: ocv1.ClusterExtensionStatus{ - Conditions: []metav1.Condition{ - {Type: "Ready", Status: metav1.ConditionTrue}, - }, - }, - }, - b: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - Finalizers: []string{"finalizer2"}, // Different finalizer should not trigger change - }, - Spec: ocv1.ClusterExtensionSpec{ - Source: ocv1.SourceConfig{ - SourceType: "Catalog", - }, - }, - Status: ocv1.ClusterExtensionStatus{ - Conditions: []metav1.Condition{ - {Type: "Ready", Status: metav1.ConditionFalse}, // Different status should not trigger change - }, - }, - }, - expected: false, - }, - { - name: "annotation changed", - a: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value1"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterExtensionSpec{}, - }, - b: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value2"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterExtensionSpec{}, - }, - expected: true, - }, - { - name: "label changed", - a: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value1"}, - }, - Spec: ocv1.ClusterExtensionSpec{}, - }, - b: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value2"}, - }, - Spec: ocv1.ClusterExtensionSpec{}, - }, - expected: true, - }, - { - name: "spec changed", - a: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterExtensionSpec{ - Source: ocv1.SourceConfig{ - SourceType: "Catalog", - }, - }, - }, - b: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterExtensionSpec{ - Source: ocv1.SourceConfig{ - SourceType: "Image", - }, - }, - }, - expected: true, - }, - { - name: "status changed but annotations, labels, spec same", - a: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterExtensionSpec{}, - Status: ocv1.ClusterExtensionStatus{ - Conditions: []metav1.Condition{ - {Type: "Ready", Status: metav1.ConditionTrue}, - }, - }, - }, - b: ocv1.ClusterExtension{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterExtensionSpec{}, - Status: ocv1.ClusterExtensionStatus{ - Conditions: []metav1.Condition{ - {Type: "Ready", Status: metav1.ConditionFalse}, - }, - }, - }, - expected: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := CheckForUnexpectedFieldChange(&tt.a, &tt.b) - if result != tt.expected { - t.Errorf("CheckForUnexpectedFieldChange() = %v, want %v", result, tt.expected) - } - }) - } -} - -func TestCheckForUnexpectedFieldChangeWithClusterCatalog(t *testing.T) { - tests := []struct { - name string - a ocv1.ClusterCatalog - b ocv1.ClusterCatalog - expected bool - }{ - { - name: "no changes", - a: ocv1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterCatalogSpec{ - Source: ocv1.CatalogSource{ - Type: "Image", - }, - }, - }, - b: ocv1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterCatalogSpec{ - Source: ocv1.CatalogSource{ - Type: "Image", - }, - }, - }, - expected: false, - }, - { - name: "spec changed", - a: ocv1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterCatalogSpec{ - Source: ocv1.CatalogSource{ - Type: "Image", - }, - }, - }, - b: ocv1.ClusterCatalog{ - ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"key": "value"}, - Labels: map[string]string{"label": "value"}, - }, - Spec: ocv1.ClusterCatalogSpec{ - Source: ocv1.CatalogSource{ - Type: "Git", - }, - }, - }, - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := CheckForUnexpectedFieldChange(&tt.a, &tt.b) - if result != tt.expected { - t.Errorf("CheckForUnexpectedFieldChange() = %v, want %v", result, tt.expected) - } - }) - } -} diff --git a/manifests/experimental-e2e.yaml b/manifests/experimental-e2e.yaml index e536cd72a..672830225 100644 --- a/manifests/experimental-e2e.yaml +++ b/manifests/experimental-e2e.yaml @@ -87,40 +87,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: catalogd-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: catalogd - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental-e2e -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: catalogd-controller-manager ---- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: operator-controller-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental-e2e -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: operator-controller-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount @@ -650,13 +616,8 @@ spec: name: v1 schema: openAPIV3Schema: - description: |- - ClusterExtensionRevision represents an immutable snapshot of Kubernetes objects - for a specific version of a ClusterExtension. Each revision contains objects - organized into phases that roll out sequentially. The same object can only be managed by a single revision - at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded - or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for - posterity. + description: ClusterExtensionRevision is the Schema for the clusterextensionrevisions + API properties: apiVersion: description: |- @@ -676,100 +637,53 @@ spec: metadata: type: object spec: - description: spec defines the desired state of the ClusterExtensionRevision. + description: spec is an optional field that defines the desired state + of the ClusterExtension. properties: lifecycleState: default: Active - description: |- - lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. - - When set to "Active" (the default), the revision is actively managed and reconciled. - When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. - The revision is removed from the owner list of all objects previously under management. - All objects that did not transition to a succeeding revision are deleted. - - Once a revision is set to "Archived", it cannot be un-archived. + description: Specifies the lifecycle state of the ClusterExtensionRevision. enum: - Active + - Paused - Archived type: string x-kubernetes-validations: - - message: cannot un-archive - rule: oldSelf == 'Active' || oldSelf == 'Archived' && oldSelf == - self + - message: can not un-archive + rule: oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' + && oldSelf == self phases: description: |- - phases is an optional, immutable list of phases that group objects to be applied together. - - Objects are organized into phases based on their Group-Kind. Common phases include: - - namespaces: Namespace objects - - policies: ResourceQuota, LimitRange, NetworkPolicy objects - - rbac: ServiceAccount, Role, RoleBinding, ClusterRole, ClusterRoleBinding objects - - crds: CustomResourceDefinition objects - - storage: PersistentVolume, PersistentVolumeClaim, StorageClass objects - - deploy: Deployment, StatefulSet, DaemonSet, Service, ConfigMap, Secret objects - - publish: Ingress, APIService, Route, Webhook objects - - All objects in a phase are applied in no particular order. - The revision progresses to the next phase only after all objects in the current phase pass their readiness probes. - - Once set, even if empty, the phases field is immutable. + Phases are groups of objects that will be applied at the same time. + All objects in the phase will have to pass their probes in order to progress to the next phase. items: description: |- - ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered - complete only after all objects pass their status probes. + ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time. + All objects in the a phase will have to pass their probes in order to progress to the next phase. properties: name: - description: |- - name is a required identifier for this phase. - - phase names must follow the DNS label standard as defined in [RFC 1123]. - They must contain only lowercase alphanumeric characters or hyphens (-), - start and end with an alphanumeric character, and be no longer than 63 characters. - - Common phase names include: namespaces, policies, rbac, crds, storage, deploy, publish. - - [RFC 1123]: https://tools.ietf.org/html/rfc1123 + description: Name identifies this phase. maxLength: 63 pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ type: string objects: - description: |- - objects is a required list of all Kubernetes objects that belong to this phase. - - All objects in this list are applied to the cluster in no particular order. + description: Objects are a list of all the objects within this + phase. items: - description: |- - ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part - of a phase, along with its collision protection settings. + description: ClusterExtensionRevisionObject contains an object + and settings for it. properties: collisionProtection: default: Prevent description: |- - collisionProtection controls whether the operator can adopt and modify objects - that already exist on the cluster. - - When set to "Prevent" (the default), the operator only manages objects it created itself. - This prevents ownership collisions. - - When set to "IfNoController", the operator can adopt and modify pre-existing objects - that are not owned by another controller. - This is useful for taking over management of manually-created resources. - - When set to "None", the operator can adopt and modify any pre-existing object, even if - owned by another controller. - Use this setting with extreme caution as it may cause multiple controllers to fight over - the same resource, resulting in increased load on the API server and etcd. + CollisionProtection controls whether OLM can adopt and modify objects + already existing on the cluster or even owned by another controller. enum: - Prevent - IfNoController - None type: string object: - description: |- - object is a required embedded Kubernetes object to be applied. - - This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true @@ -790,12 +704,9 @@ spec: rule: self == oldSelf || oldSelf.size() == 0 revision: description: |- - revision is a required, immutable sequence number representing a specific revision - of the parent ClusterExtension. - - The revision field must be a positive integer. - Each ClusterExtensionRevision belonging to the same parent ClusterExtension must have a unique revision number. - The revision number must always be the previous revision number plus one, or 1 for the first revision. + Revision is a sequence number representing a specific revision of the ClusterExtension instance. + Must be positive. Each ClusterExtensionRevision of the same parent ClusterExtension needs to have + a unique value assigned. It is immutable after creation. The new revision number must always be previous revision +1. format: int64 minimum: 1 type: integer @@ -806,31 +717,10 @@ spec: - revision type: object status: - description: status is optional and defines the observed state of the - ClusterExtensionRevision. + description: status is an optional field that defines the observed state + of the ClusterExtension. properties: conditions: - description: |- - conditions is an optional list of status conditions describing the state of the - ClusterExtensionRevision. - - The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - - When Progressing is not present, the revision is not currently in transition. - - The Available condition represents whether the revision has been successfully rolled out and is available: - - When status is True and reason is Available, the revision has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. - - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - - When status is False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. - - The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/manifests/experimental.yaml b/manifests/experimental.yaml index f88debab0..199838eac 100644 --- a/manifests/experimental.yaml +++ b/manifests/experimental.yaml @@ -87,40 +87,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: catalogd-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: catalogd - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: catalogd-controller-manager ---- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: operator-controller-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: operator-controller-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount @@ -615,13 +581,8 @@ spec: name: v1 schema: openAPIV3Schema: - description: |- - ClusterExtensionRevision represents an immutable snapshot of Kubernetes objects - for a specific version of a ClusterExtension. Each revision contains objects - organized into phases that roll out sequentially. The same object can only be managed by a single revision - at a time. Ownership of objects is transitioned from one revision to the next as the extension is upgraded - or reconfigured. Once the latest revision has rolled out successfully, previous active revisions are archived for - posterity. + description: ClusterExtensionRevision is the Schema for the clusterextensionrevisions + API properties: apiVersion: description: |- @@ -641,100 +602,53 @@ spec: metadata: type: object spec: - description: spec defines the desired state of the ClusterExtensionRevision. + description: spec is an optional field that defines the desired state + of the ClusterExtension. properties: lifecycleState: default: Active - description: |- - lifecycleState specifies the lifecycle state of the ClusterExtensionRevision. - - When set to "Active" (the default), the revision is actively managed and reconciled. - When set to "Archived", the revision is inactive and any resources not managed by a subsequent revision are deleted. - The revision is removed from the owner list of all objects previously under management. - All objects that did not transition to a succeeding revision are deleted. - - Once a revision is set to "Archived", it cannot be un-archived. + description: Specifies the lifecycle state of the ClusterExtensionRevision. enum: - Active + - Paused - Archived type: string x-kubernetes-validations: - - message: cannot un-archive - rule: oldSelf == 'Active' || oldSelf == 'Archived' && oldSelf == - self + - message: can not un-archive + rule: oldSelf == 'Active' || oldSelf == 'Paused' || oldSelf == 'Archived' + && oldSelf == self phases: description: |- - phases is an optional, immutable list of phases that group objects to be applied together. - - Objects are organized into phases based on their Group-Kind. Common phases include: - - namespaces: Namespace objects - - policies: ResourceQuota, LimitRange, NetworkPolicy objects - - rbac: ServiceAccount, Role, RoleBinding, ClusterRole, ClusterRoleBinding objects - - crds: CustomResourceDefinition objects - - storage: PersistentVolume, PersistentVolumeClaim, StorageClass objects - - deploy: Deployment, StatefulSet, DaemonSet, Service, ConfigMap, Secret objects - - publish: Ingress, APIService, Route, Webhook objects - - All objects in a phase are applied in no particular order. - The revision progresses to the next phase only after all objects in the current phase pass their readiness probes. - - Once set, even if empty, the phases field is immutable. + Phases are groups of objects that will be applied at the same time. + All objects in the phase will have to pass their probes in order to progress to the next phase. items: description: |- - ClusterExtensionRevisionPhase represents a group of objects that are applied together. The phase is considered - complete only after all objects pass their status probes. + ClusterExtensionRevisionPhase are groups of objects that will be applied at the same time. + All objects in the a phase will have to pass their probes in order to progress to the next phase. properties: name: - description: |- - name is a required identifier for this phase. - - phase names must follow the DNS label standard as defined in [RFC 1123]. - They must contain only lowercase alphanumeric characters or hyphens (-), - start and end with an alphanumeric character, and be no longer than 63 characters. - - Common phase names include: namespaces, policies, rbac, crds, storage, deploy, publish. - - [RFC 1123]: https://tools.ietf.org/html/rfc1123 + description: Name identifies this phase. maxLength: 63 pattern: ^[a-z]([-a-z0-9]*[a-z0-9])?$ type: string objects: - description: |- - objects is a required list of all Kubernetes objects that belong to this phase. - - All objects in this list are applied to the cluster in no particular order. + description: Objects are a list of all the objects within this + phase. items: - description: |- - ClusterExtensionRevisionObject represents a Kubernetes object to be applied as part - of a phase, along with its collision protection settings. + description: ClusterExtensionRevisionObject contains an object + and settings for it. properties: collisionProtection: default: Prevent description: |- - collisionProtection controls whether the operator can adopt and modify objects - that already exist on the cluster. - - When set to "Prevent" (the default), the operator only manages objects it created itself. - This prevents ownership collisions. - - When set to "IfNoController", the operator can adopt and modify pre-existing objects - that are not owned by another controller. - This is useful for taking over management of manually-created resources. - - When set to "None", the operator can adopt and modify any pre-existing object, even if - owned by another controller. - Use this setting with extreme caution as it may cause multiple controllers to fight over - the same resource, resulting in increased load on the API server and etcd. + CollisionProtection controls whether OLM can adopt and modify objects + already existing on the cluster or even owned by another controller. enum: - Prevent - IfNoController - None type: string object: - description: |- - object is a required embedded Kubernetes object to be applied. - - This object must be a valid Kubernetes resource with apiVersion, kind, and metadata fields. type: object x-kubernetes-embedded-resource: true x-kubernetes-preserve-unknown-fields: true @@ -755,12 +669,9 @@ spec: rule: self == oldSelf || oldSelf.size() == 0 revision: description: |- - revision is a required, immutable sequence number representing a specific revision - of the parent ClusterExtension. - - The revision field must be a positive integer. - Each ClusterExtensionRevision belonging to the same parent ClusterExtension must have a unique revision number. - The revision number must always be the previous revision number plus one, or 1 for the first revision. + Revision is a sequence number representing a specific revision of the ClusterExtension instance. + Must be positive. Each ClusterExtensionRevision of the same parent ClusterExtension needs to have + a unique value assigned. It is immutable after creation. The new revision number must always be previous revision +1. format: int64 minimum: 1 type: integer @@ -771,31 +682,10 @@ spec: - revision type: object status: - description: status is optional and defines the observed state of the - ClusterExtensionRevision. + description: status is an optional field that defines the observed state + of the ClusterExtension. properties: conditions: - description: |- - conditions is an optional list of status conditions describing the state of the - ClusterExtensionRevision. - - The Progressing condition represents whether the revision is actively rolling out: - - When status is True and reason is Progressing, the revision rollout is actively making progress and is in transition. - - When Progressing is not present, the revision is not currently in transition. - - The Available condition represents whether the revision has been successfully rolled out and is available: - - When status is True and reason is Available, the revision has been successfully rolled out and all objects pass their readiness probes. - - When status is False and reason is Incomplete, the revision rollout has not yet completed but no specific failures have been detected. - - When status is False and reason is ProbeFailure, one or more objects are failing their readiness probes during rollout. - - When status is False and reason is ReconcileFailure, the revision has encountered a general reconciliation failure. - - When status is False and reason is RevisionValidationFailure, the revision failed preflight validation checks. - - When status is False and reason is PhaseValidationError, a phase within the revision failed preflight validation checks. - - When status is False and reason is ObjectCollisions, objects in the revision collide with existing cluster objects that cannot be adopted. - - When status is Unknown and reason is Archived, the revision has been archived and its objects have been torn down. - - When status is Unknown and reason is Migrated, the revision was migrated from an existing release and object status probe results have not yet been observed. - - The Succeeded condition represents whether the revision has successfully completed its rollout: - - When status is True and reason is RolloutSuccess, the revision has successfully completed its rollout. This condition is set once and persists even if the revision later becomes unavailable. items: description: Condition contains details for one aspect of the current state of this API Resource. diff --git a/manifests/standard-e2e.yaml b/manifests/standard-e2e.yaml index 1aed38ba9..5c9590784 100644 --- a/manifests/standard-e2e.yaml +++ b/manifests/standard-e2e.yaml @@ -87,40 +87,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: catalogd-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: catalogd - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard-e2e -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: catalogd-controller-manager ---- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: operator-controller-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard-e2e -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: operator-controller-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount diff --git a/manifests/standard.yaml b/manifests/standard.yaml index 34cc57918..95e400c26 100644 --- a/manifests/standard.yaml +++ b/manifests/standard.yaml @@ -87,40 +87,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: catalogd-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: catalogd - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: catalogd-controller-manager ---- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: operator-controller-controller-manager - namespace: olmv1-system - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: operator-controller-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount diff --git a/openshift/catalogd/manifests-experimental.yaml b/openshift/catalogd/manifests-experimental.yaml index 06bd2abd8..4dacdee86 100644 --- a/openshift/catalogd/manifests-experimental.yaml +++ b/openshift/catalogd/manifests-experimental.yaml @@ -65,23 +65,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: catalogd-controller-manager - namespace: openshift-catalogd - labels: - app.kubernetes.io/name: catalogd - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: catalogd-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount diff --git a/openshift/catalogd/manifests.yaml b/openshift/catalogd/manifests.yaml index e197256bf..68b6c87f3 100644 --- a/openshift/catalogd/manifests.yaml +++ b/openshift/catalogd/manifests.yaml @@ -65,23 +65,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-catalogd.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: catalogd-controller-manager - namespace: openshift-catalogd - labels: - app.kubernetes.io/name: catalogd - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: catalogd-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount diff --git a/openshift/operator-controller/manifests-experimental.yaml b/openshift/operator-controller/manifests-experimental.yaml index e8893063e..6ecb52ff2 100644 --- a/openshift/operator-controller/manifests-experimental.yaml +++ b/openshift/operator-controller/manifests-experimental.yaml @@ -61,23 +61,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: operator-controller-controller-manager - namespace: openshift-operator-controller - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: experimental -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: operator-controller-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount diff --git a/openshift/operator-controller/manifests.yaml b/openshift/operator-controller/manifests.yaml index 8d2be5ecf..091dfe26a 100644 --- a/openshift/operator-controller/manifests.yaml +++ b/openshift/operator-controller/manifests.yaml @@ -61,23 +61,6 @@ spec: - Ingress - Egress --- -# Source: olmv1/templates/poddisruptionbudget-olmv1-system-operator-controller.yml -apiVersion: policy/v1 -kind: PodDisruptionBudget -metadata: - name: operator-controller-controller-manager - namespace: openshift-operator-controller - labels: - app.kubernetes.io/name: operator-controller - app.kubernetes.io/part-of: olm - annotations: - olm.operatorframework.io/feature-set: standard -spec: - minAvailable: 1 - selector: - matchLabels: - control-plane: operator-controller-controller-manager ---- # Source: olmv1/templates/serviceaccount-olmv1-system-common-controller-manager.yml apiVersion: v1 kind: ServiceAccount diff --git a/requirements.txt b/requirements.txt index ba058cbbc..72686f75c 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,7 +21,7 @@ paginate==0.5.7 pathspec==0.12.1 platformdirs==4.5.0 Pygments==2.19.2 -pymdown-extensions==10.17.2 +pymdown-extensions==10.17.1 pyquery==2.0.1 python-dateutil==2.9.0.post0 PyYAML==6.0.3 diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index b3380ff0f..ab0bf48b1 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "slices" "testing" "time" @@ -198,8 +197,7 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) { t.Log("When a cluster extension is installed from a catalog") clusterExtension, extensionCatalog, sa, ns := TestInit(t) - extraCatalogName := fmt.Sprintf("extra-test-catalog-%s", rand.String(8)) - extraCatalog, err := CreateTestCatalog(context.Background(), extraCatalogName, os.Getenv(testCatalogRefEnvVar)) + extraCatalog, err := CreateTestCatalog(context.Background(), "extra-test-catalog", os.Getenv(testCatalogRefEnvVar)) require.NoError(t, err) defer TestCleanup(t, extensionCatalog, clusterExtension, sa, ns) @@ -240,11 +238,7 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) { require.NotNil(ct, cond) require.Equal(ct, metav1.ConditionTrue, cond.Status) require.Equal(ct, ocv1.ReasonRetrying, cond.Reason) - // Catalog names are sorted alphabetically in the error message - catalogs := []string{extensionCatalog.Name, extraCatalog.Name} - slices.Sort(catalogs) - expectedMessage := fmt.Sprintf("in multiple catalogs with the same priority %v", catalogs) - require.Contains(ct, cond.Message, expectedMessage) + require.Contains(ct, cond.Message, "in multiple catalogs with the same priority [extra-test-catalog test-catalog]") }, pollDuration, pollInterval) } @@ -447,7 +441,7 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) { // patch imageRef tag on test-catalog image with v2 image t.Log("By patching the catalog ImageRef to point to the v2 catalog") updatedCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:v2", os.Getenv("CLUSTER_REGISTRY_HOST")) - err := patchTestCatalog(context.Background(), extensionCatalog.Name, updatedCatalogImage) + err := patchTestCatalog(context.Background(), testCatalogName, updatedCatalogImage) require.NoError(t, err) require.EventuallyWithT(t, func(ct *assert.CollectT) { require.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: extensionCatalog.Name}, extensionCatalog)) @@ -480,9 +474,8 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) { require.NoError(t, err) // create a test-catalog with latest image tag - catalogName := fmt.Sprintf("test-catalog-%s", rand.String(8)) latestCatalogImage := fmt.Sprintf("%s/e2e/test-catalog:latest", os.Getenv("CLUSTER_REGISTRY_HOST")) - extensionCatalog, err := CreateTestCatalog(context.Background(), catalogName, latestCatalogImage) + extensionCatalog, err := CreateTestCatalog(context.Background(), testCatalogName, latestCatalogImage) require.NoError(t, err) clusterExtensionName := fmt.Sprintf("clusterextension-%s", rand.String(8)) clusterExtension := &ocv1.ClusterExtension{ diff --git a/test/helpers/helpers.go b/test/helpers/helpers.go index af142c6e3..49ebeaab6 100644 --- a/test/helpers/helpers.go +++ b/test/helpers/helpers.go @@ -218,7 +218,6 @@ func TestInit(t *testing.T) (*ocv1.ClusterExtension, *ocv1.ClusterCatalog, *core func TestInitClusterExtensionClusterCatalog(t *testing.T) (*ocv1.ClusterExtension, *ocv1.ClusterCatalog) { ceName := fmt.Sprintf("clusterextension-%s", rand.String(8)) - catalogName := fmt.Sprintf("test-catalog-%s", rand.String(8)) ce := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ @@ -226,10 +225,10 @@ func TestInitClusterExtensionClusterCatalog(t *testing.T) (*ocv1.ClusterExtensio }, } - cc, err := CreateTestCatalog(context.Background(), catalogName, os.Getenv(testCatalogRefEnvVar)) + cc, err := CreateTestCatalog(context.Background(), testCatalogName, os.Getenv(testCatalogRefEnvVar)) require.NoError(t, err) - ValidateCatalogUnpackWithName(t, catalogName) + ValidateCatalogUnpack(t) return ce, cc } @@ -251,18 +250,11 @@ func TestInitServiceAccountNamespace(t *testing.T, clusterExtensionName string) return sa, ns } -// ValidateCatalogUnpack validates that the test catalog with the default name has unpacked successfully. -// Deprecated: Use ValidateCatalogUnpackWithName for tests that use unique catalog names. func ValidateCatalogUnpack(t *testing.T) { - ValidateCatalogUnpackWithName(t, testCatalogName) -} - -// ValidateCatalogUnpackWithName validates that a catalog with the given name has unpacked successfully. -func ValidateCatalogUnpackWithName(t *testing.T, catalogName string) { catalog := &ocv1.ClusterCatalog{} t.Log("Ensuring ClusterCatalog has Status.Condition of Progressing with a status == True and reason == Succeeded") require.EventuallyWithT(t, func(ct *assert.CollectT) { - err := c.Get(context.Background(), types.NamespacedName{Name: catalogName}, catalog) + err := c.Get(context.Background(), types.NamespacedName{Name: testCatalogName}, catalog) require.NoError(ct, err) cond := apimeta.FindStatusCondition(catalog.Status.Conditions, ocv1.TypeProgressing) require.NotNil(ct, cond) @@ -273,11 +265,11 @@ func ValidateCatalogUnpackWithName(t *testing.T, catalogName string) { t.Log("Checking that catalog has the expected metadata label") require.NotNil(t, catalog.Labels) require.Contains(t, catalog.Labels, "olm.operatorframework.io/metadata.name") - require.Equal(t, catalogName, catalog.Labels["olm.operatorframework.io/metadata.name"]) + require.Equal(t, testCatalogName, catalog.Labels["olm.operatorframework.io/metadata.name"]) t.Log("Ensuring ClusterCatalog has Status.Condition of Type = Serving with status == True") require.EventuallyWithT(t, func(ct *assert.CollectT) { - err := c.Get(context.Background(), types.NamespacedName{Name: catalogName}, catalog) + err := c.Get(context.Background(), types.NamespacedName{Name: testCatalogName}, catalog) require.NoError(ct, err) cond := apimeta.FindStatusCondition(catalog.Status.Conditions, ocv1.TypeServing) require.NotNil(ct, cond)