Prepare operator v2 - #472
Conversation
Bumps k8s.io/{api,apimachinery,client-go,code-generator} from v0.33.1 to
v0.36.3 and sigs.k8s.io/controller-runtime from v0.21.0 to v0.24.1, which
brings in sigs.k8s.io/structured-merge-diff/v6.
No source changes were needed: the operator builds, vets and passes the
unit suite untouched. The regenerated CRDs are included because they inline
upstream corev1 types, so bumping k8s.io/api adds new optional fields such
as env var fileKeyRef and rewords some descriptions. The change is purely
additive at the schema level - no property or validation was removed - so
existing custom resources are unaffected.
Beyond staying current, this unblocks using the typed sigs.k8s.io/gateway-api
SDK, which could not be added while controller-runtime v0.21 pinned
k8s.io/apimachinery to v0.33 and therefore structured-merge-diff/v4.
Ref: SEC-11130
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MattermostRestoreDB restored a MySQL backup from a bucket into a database provisioned by the MySQL operator. It only ever worked against database.operatorManaged, which is being removed, so the resource has no remaining purpose. It was also tied to the legacy API: its spec field is documented as "MattermostClusterName defines the ClusterInstallation name", so it only addressed ClusterInstallation resources, never Mattermost ones. BREAKING CHANGE: the mattermostrestoredbs.mattermost.com CRD is removed. Any remaining MattermostRestoreDB objects should be deleted before upgrading; after the CRD is gone they are unreadable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ClusterInstallation was the Operator's original custom resource, superseded by Mattermost (installation.mattermost.com/v1beta1). Its controller carried a large amount of machinery that has no equivalent in the current resource: blue-green and canary deployment strategies, a MySQL and a PostgreSQL path, MinIO provisioning, and the conversion and migration code that produced a Mattermost from a ClusterInstallation. With both of its custom resources now gone, the mattermost.com API group is empty, so the Operator's ClusterRole no longer requests any permission on it. The apis/mattermost/v1alpha1 Go package is deliberately left in place for now: v1beta1 still imports it for the spec.size presets. Those presets move to v1beta1 and the package is deleted in a later commit, which also removes the generated v1alpha1 clientset, listers and informers. BREAKING CHANGE: the clusterinstallations.mattermost.com CRD is removed, and with it the automatic ClusterInstallation to Mattermost migration. Anyone still running ClusterInstallation resources must upgrade to Operator 1.24.x first and let it complete the migration, then upgrade to this release. Upgrading directly leaves ClusterInstallation objects unreadable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Operator provisioned an in-cluster MinIO by creating a MinIOInstance for
the MinIO operator to act on. That integration is no longer viable:
- github.com/minio/minio-operator, the module supplying the MinIOInstance
type, was last published in February 2020 and the repository is archived.
- miniocontroller.min.io/v1beta1 MinIOInstance is not served by any current
MinIO operator, which moved to minio.min.io/v2 Tenant. The path has been
incompatible with a supported MinIO operator for years.
- The MinIOInstance was created without an image, so the MinIO server
version was whatever minio/k8s-operator:1.0.7 defaulted to. That server
held all user uploads and was invisible to image scanning, since no tag
appeared anywhere in this repository.
Removing it also drops two init containers from every Mattermost pod in that
mode, one of which ran appropriate/curl:latest - an abandoned image on a
mutable tag.
External S3 support is unaffected. fileStore.external still targets any
S3 compatible endpoint, including a MinIO deployment managed outside the
Operator, and resources/minio.yaml is retained because the e2e-external suite
uses it as exactly that.
BREAKING CHANGE: spec.fileStore.operatorManaged is removed, and a file store
is now mandatory. Until now a Mattermost with no fileStore section silently
received an in-cluster MinIO; such a resource is now rejected with an error
asking for fileStore.external, fileStore.local or fileStore.externalVolume.
There is deliberately no replacement default. Defaulting to local storage
would repoint an installation at an empty PVC while its files remained in the
old MinIO, so this fails loudly instead of losing data quietly.
Existing MinIO instances are left untouched: nothing in this change deletes a
MinIOInstance. Copy the objects out to S3 or into a PVC and set the matching
fileStore before upgrading.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Operator provisioned a database by creating a MysqlCluster for the presslabs MySQL operator to act on, using a copy of that operator's API types vendored into pkg/database/mysql_operator. Combined with the MinIO removal in the previous commit, this leaves the Operator with no dependency on any third-party operator: its ClusterRole now requests permissions only on core Kubernetes groups plus installation.mattermost.com. Removed alongside it: pkg/components/mysql, pkg/resources/mysql.go, the MySQLDBConfig database config, the mysql-operator install script and Makefile target, the bitpoke image pulls in the e2e setup script, and the vendored presslabs types. The stale "git checkout pkg/database/mysql_operator/*" guard in the generate target went with them. External databases are unaffected. database.external continues to take a secret holding a connection string, which is how every Mattermost Cloud installation is already configured. BREAKING CHANGE: spec.database.operatorManaged is removed, and a database is now mandatory. A Mattermost with no database section previously received a MySQL cluster provisioned by the MySQL operator; such a resource is now rejected with an error asking for database.external. As with the file store, there is deliberately no replacement default. Existing MysqlCluster objects are left untouched: nothing here deletes one. Dump the database, load it into a database you manage, and point database.external at a secret with its connection string before upgrading. NOTE: the test/e2e suite provisioned its database through the MySQL operator, so testDatabaseConfig now returns an external database using the db-credentials secret from resources/postgres.yaml, matching the e2e-external suite. The MySQL StatefulSet waits and the MySQL-replicas test are gone. That suite needs a cluster to run and has not been exercised here, so it should be validated in CI before release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With the ClusterInstallation and MattermostRestoreDB resources gone, nothing
served the mattermost.com/v1alpha1 API any more. The package survived only
because v1beta1 imported it for the spec.size presets, so those move first.
The presets shrink considerably in the process. ClusterInstallationSize held
App, Minio and Database component sizes; with both Operator-managed workloads
removed only App remains, so the type is now simply Size{App ComponentSize}
and CalculateResourceMilliRequirements loses its includeDatabase and
includeMinio parameters. All nine size keys keep their existing app server
replica counts and resource requests, so spec.size behaves identically.
Also removed:
- pkg/mattermost/mattermost.go, the v1alpha1 resource generators, dead since
its only caller was the ClusterInstallation controller. Two shared values
it happened to hold, SetupJobName and defaultIngressPathType, moved to
mattermost_v1beta.go, and its env var test helpers moved to a dedicated
testhelpers_test.go.
- the generated v1alpha1 clientset, listers and informers under pkg/client,
along with the three generate-groups.sh invocations and the openapi-gen
call that produced them. The v1beta1 client under pkg/client/v1beta1 is
untouched.
BREAKING CHANGE: this is a Go API break for anything importing
apis/mattermost/v1alpha1 directly, notably mattermost-cloud. The size presets
are now in apis/mattermost/v1beta1 as Size rather than ClusterInstallationSize,
and the Calculate* helpers no longer take component flags.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bumps the version to 2.0.0 and brings the documentation in line with what the Operator now does. The README's forward-looking "Migrate to Mattermost Custom Resource" section is replaced by an upgrade guide describing what v2.0.0 removed and, more importantly, the three things a user may have to do before upgrading: complete the ClusterInstallation migration on v1.24.x, move an Operator-managed database to one they manage, and copy objects out of an Operator-managed MinIO. The section documenting MattermostRestoreDB is gone with the resource. docs/migration.md keeps its ClusterInstallation to Mattermost instructions, since users on older Operators still need them, but now opens with a warning that the migration has to be run on v1.24.x or earlier because v2.0.0 no longer contains the code. Examples: the two ClusterInstallation manifests are removed, and the two "simple" Mattermost examples configured neither a database nor a file store, so they would have been rejected by the new validation. They now show an external database plus, respectively, local PVC storage and an S3 bucket. The CRD reference doc drops the removed OperatorManagedDatabase and OperatorManagedMinio types. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixed 5 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Addresses two review findings. Size presets were aliased rather than copied. setDefaultReplicasAndResources assigned &DefaultSize.App.Replicas, handing every Mattermost a pointer into the package-level preset, and both it and overrideReplicasAndResourcesFromSize assigned ResourceRequirements directly, whose ResourceLists are maps and so were shared by reference. Scaling one installation therefore rewrote the preset and changed the defaults every installation reconciled afterwards received. Demonstrated before fixing: mutating one Mattermost moved DefaultSize.App.Replicas from 2 to 999 and its CPU request from 500m to 777m, and a second, unrelated Mattermost then defaulted to those values. Both paths now copy, with regression tests that fail against the previous code. This predates the v2 work - the v1alpha1 implementation aliased the presets the same way - but the code moved to v1beta1 in this branch, so it is fixed here. The e2e suite was missing the prerequisites it can no longer expect the Operator to create. Two problems: test/e2e/setup.go still listed test/crds in CRDDirectoryPaths, a directory removed with the MinIO and MySQL operator CRDs, which would fail envtest startup; and nothing provisioned the database and file store that are now mandatory. CreateFromFile moves from test/e2e-external to test/e2e, which e2e-external already imports, and a new SetupMattermostPrerequisites applies the same postgres.yaml and mm-secrets.yaml fixtures that suite uses. The e2e suite still requires a real cluster and has not been run here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit pointed the e2e suite's file store at local (PVC backed)
storage. That cannot work in this suite for two reasons:
- New local file stores request ReadWriteMany, which the local-path
provisioner in a kind cluster cannot satisfy, so the PVC would never bind.
- Even forced to ReadWriteOnce, the scale test scales to two replicas, and a
single RWO volume cannot be mounted by two pods.
The suite now uses an external file store pointing at the standalone MinIO in
resources/minio.yaml, which is exactly what the e2e-external suite does:
minio:9000, test-bucket, file-store-credentials. That fixture is applied by
SetupMattermostPrerequisites alongside postgres.yaml and mm-secrets.yaml. It
carries its own root credentials Secret, whose MINIO_ROOT_USER matches the
accesskey in file-store-credentials.
This keeps resources/minio.yaml earning its place: despite the name it has
always been a standalone MinIO used as an external S3 endpoint for testing,
never the Operator-managed integration that was removed.
The suite still requires a real cluster and has not been run here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The auto-fix commit and the two local commits addressed the same three review
findings independently. Resolution, after rebasing the local commits on top:
Kept from the auto-fix:
- the docs/examples size comment, which fixes a typo and corrects
"limits for the database and app servers" to "for the app servers". The
earlier local edit had left the database in that sentence even after
database sizing was removed along with the presets' Database component.
- the mattermost_sizes.go change, which is byte-identical to the local fix.
Kept locally:
- the e2e prerequisite setup. The auto-fix added a second copy of
CreateFromFile to test/e2e/utils.go while the local commit had moved the
original out of test/e2e-external, so both existed in package e2e and the
symbol was redeclared. The moved copy is kept, since e2e-external reuses it
rather than duplicating it.
- SetupMattermostPrerequisites rather than provisionTestPrerequisites. The
latter did not compile: it referenced mmNamespace, which is declared in
mattermost_test.go and so is not visible from the non-test setup.go. It was
also called from the shared SetupTest, which e2e-external also uses, and
resources/postgres.yaml contains a cluster-scoped PersistentVolume, so
applying it from there would collide with the fixtures e2e-external applies
itself.
- the external MinIO file store. The auto-fix left the file store on local
storage, so the ReadWriteMany problem its own comment described was not
actually resolved.
Note the rebase reported no conflicts: the two sets of changes landed in
different hunks of the same files, so git merged them cleanly into code that did
not build. The duplicate symbols and imports only showed up on go build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds the replica assertion suggested in review, and corrects the comment to say what it does and does not guard. Checking it revealed the suggestion's premise does not hold for this path. GetClusterSize returns a Size by value and overrideReplicasAndResourcesFromSize takes one by value, so &size.App.Replicas would point into a per-call copy rather than into the preset. Regressing that line to the aliasing form and re-running the test confirms it: the test still passes, because there is nothing shared to leak. What does leak through a value copy is the maps. Copying a Size copies the ResourceList map headers, so the resource requests stay shared with the preset unless DeepCopy is used - which is what the existing assertion in this test catches. Replica aliasing is a real hazard in the other path, where setDefaultReplicasAndResources reads the package-level DefaultSize directly, and TestSizeDefaultsAreNotShared catches it: regressing that line makes it fail on both the preset and the leak into a second Mattermost. The assertion is kept for documentation, with a comment recording which path each test actually protects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ci/generate-operator-manifests job snapshots the API packages, runs the
generators and diffs the result to catch uncommitted generated code. It still
snapshotted apis/mattermost/v1alpha1, so it failed on the missing directory:
cp: cannot stat 'apis/mattermost/v1alpha1/*': No such file or directory
The v1alpha1 snapshot and diff are removed. The v1beta1 and config/crd/bases
checks are unchanged, and the job was replayed locally in a correct-GOPATH copy:
make generate manifests exits 0 and both diffs are clean.
test/setup_test.sh also pre-pulled percona:8.0 and prom/mysqld-exporter into the
kind cluster. Those images existed only for the Operator-managed MySQL path, so
they are no longer used by anything and were costing the e2e job a needless
image pull. The e2e suites provision Postgres and MinIO from resources/
themselves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CreateFromFile returns once the objects are created, not once they serve traffic, so SetupMattermostPrerequisites returned while both Deployments were still starting and the suite immediately created Mattermost resources against them. This matters more than ordering tidiness. mm-secrets.yaml carries DB_CONNECTION_STRING but no DB_CONNECTION_CHECK_URL, and NewExternalDBConfig only injects the database readiness init container when that key is present (database_external.go:51). Nothing else in the reconcile blocks on the database being reachable, so the Mattermost pods started against a Postgres that was not listening and recovered only through crash-loop backoff - slow, and dependent on timing rather than on anything the suite guarantees. SetupMattermostPrerequisites now polls both the postgresql and minio Deployments for an available replica, with a five minute ceiling, and tears the fixtures back down if either never arrives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
make deploy failed applying the CRD:
The CustomResourceDefinition "mattermosts.installation.mattermost.com" is
invalid: metadata.annotations: Too long: must have at most 262144 bytes
kubectl apply stores the applied manifest in
kubectl.kubernetes.io/last-applied-configuration, and that annotation is capped
at 262144 bytes. Measured as JSON, which is what the annotation holds:
master 260965 -1179 under
10f2765 k8s v0.36 bump 272647 +10503 over
a1c562c MinIO removal 269846 +7702 over
6d55e3e MySQL removal 266340 +4196 over
master was 1179 bytes, 0.45%, below the ceiling. The Kubernetes library upgrade
pushed it over by inlining new corev1 fields such as env var fileKeyRef into the
schema; removing the Operator-managed database and file store recovered 6.3KB of
that but not enough.
CRD_OPTIONS now passes crd:maxDescLen=200, which truncates field descriptions and
brings the CRD to 192749 bytes as JSON, 69KB clear of the limit. That headroom is
the point: a routine dependency bump consumed the previous margin, and the same
would happen again at a smaller setting.
Truncation only affects what kubectl explain prints. Short descriptions are
unchanged, long ones keep their opening sentences, and the full field
documentation remains in docs/mattermost_v1beta1_crd.md, which is generated
separately and unaffected.
Server-side apply is the other way to avoid the annotation entirely, but it would
change the documented install procedure for every user and require
--force-conflicts when upgrading over a client-side applied install. Keeping plain
kubectl apply working is preferable in a release that already asks users to
migrate their database and file store.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR prepares Mattermost Operator v2.0.0 by removing the deprecated v1alpha1 APIs and all operator-managed dependency integrations (MySQL/MinIO), updating Kubernetes/controller-runtime dependencies, and refactoring tests/docs accordingly to require explicitly configured external database + file store.
Changes:
- Bump Operator version to 2.0.0, upgrade
k8s.io/*to v0.36.3 andcontroller-runtimeto v0.24.1, and adjust CRD generation options to avoid annotation size limits. - Remove
ClusterInstallationandMattermostRestoreDBCRDs/controllers, delete vendored Presslabs MySQL operator API types + MinIO operator integration, and prune RBAC accordingly. - Update e2e and unit tests + examples/docs to reflect the new requirement for external DB + file store (no silent operator-managed defaults).
Reviewed changes
Copilot reviewed 119 out of 127 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| version/version.go | Bumps operator version string to 2.0.0. |
| test/setup_test.sh | Removes pre-pulling/loading MySQL/MinIO operator images for kind e2e. |
| test/e2e/utils.go | Drops MySQL-operator readiness helper and related imports. |
| test/e2e/setup.go | Removes external-operator CRDs from envtest and adds prerequisite provisioning helpers. |
| test/e2e/mattermost_test.go | Updates e2e flow to provision external DB/S3 fixtures and removes MySQL-replica test. |
| test/e2e/apply_resource.go | Adds helper to create test resources from YAML fixtures. |
| test/e2e-external/size_test.go | Migrates size preset usage from v1alpha1 to v1beta1. |
| test/e2e-external/setup_external_resources.go | Reuses shared e2e.CreateFromFile for external fixture setup. |
| test/crds/minio_crd.yaml | Removes MinIO operator CRD from test assets. |
| scripts/install-mysql-minio.sh | Removes script that installed MySQL/MinIO operators. |
| README.md | Rewrites upgrade guidance for v2.0.0 and removes MySQL/MinIO operator workflow references. |
| pkg/resources/mysql.go | Removes MySQL operator resource helper. |
| pkg/resources/minio.go | Removes MinIO operator resource helper. |
| pkg/mattermost/testhelpers_test.go | Extracts env var assertion helpers into a standalone test helper file. |
| pkg/mattermost/mattermost_v1beta.go | Stops referencing v1alpha1 constants; adds exported consts and default ingress path type var. |
| pkg/mattermost/mattermost_v1beta_test.go | Removes operator-managed filestore coverage and updates default DB/filestore expectations. |
| pkg/mattermost/file_store.go | Removes operator-managed MinIO config and constructor. |
| pkg/mattermost/file_store_test.go | Removes operator-managed MinIO tests; updates external filestore tests. |
| pkg/mattermost/database_mysql.go | Removes operator-managed MySQL DB config implementation. |
| pkg/mattermost/database_mysql_test.go | Removes tests for operator-managed MySQL DB config. |
| pkg/database/mysql_operator/v1alpha1/zz_generated.defaults.go | Removes vendored Presslabs MySQL operator API defaults. |
| pkg/database/mysql_operator/v1alpha1/register.go | Removes vendored Presslabs MySQL operator scheme registration. |
| pkg/database/mysql_operator/v1alpha1/mysqluser_types.go | Removes vendored Presslabs MySQL operator MysqlUser types. |
| pkg/database/mysql_operator/v1alpha1/mysqldatabase_types.go | Removes vendored Presslabs MySQL operator MysqlDatabase types. |
| pkg/database/mysql_operator/v1alpha1/mysqlcluster_defaults.go | Removes vendored Presslabs MySQL operator defaulting. |
| pkg/database/mysql_operator/v1alpha1/mysqlbackup_types.go | Removes vendored Presslabs MySQL operator MysqlBackup types. |
| pkg/database/mysql_operator/v1alpha1/mysqlbackup_defaults.go | Removes vendored Presslabs MySQL operator backup defaults. |
| pkg/database/mysql_operator/v1alpha1/doc.go | Removes vendored Presslabs MySQL operator package doc. |
| pkg/components/mysql/mysql.go | Removes MySQL component generator for v1alpha1/v1beta1 operator-managed DB paths. |
| pkg/components/minio/minio.go | Removes MinIO component generator for v1alpha1/v1beta1 operator-managed file store paths. |
| pkg/client/listers/mattermost/v1alpha1/expansion_generated.go | Removes generated v1alpha1 lister expansion. |
| pkg/client/listers/mattermost/v1alpha1/clusterinstallation.go | Removes generated v1alpha1 ClusterInstallation lister. |
| pkg/client/informers/externalversions/mattermost/v1alpha1/interface.go | Removes generated v1alpha1 informer interface. |
| pkg/client/informers/externalversions/mattermost/v1alpha1/clusterinstallation.go | Removes generated v1alpha1 ClusterInstallation informer. |
| pkg/client/informers/externalversions/mattermost/interface.go | Removes generated mattermost informer root interface (v1alpha1). |
| pkg/client/informers/externalversions/internalinterfaces/factory_interfaces.go | Removes generated informer factory interfaces used by v1alpha1 informers. |
| pkg/client/informers/externalversions/generic.go | Removes generated generic informer wiring for v1alpha1 ClusterInstallation. |
| pkg/client/informers/externalversions/factory.go | Removes generated informer factory implementation for v1alpha1. |
| pkg/client/clientset/versioned/typed/mattermost/v1alpha1/mattermost_client.go | Removes generated typed client for v1alpha1 APIs. |
| pkg/client/clientset/versioned/typed/mattermost/v1alpha1/generated_expansion.go | Removes generated expansion for v1alpha1 typed client. |
| pkg/client/clientset/versioned/typed/mattermost/v1alpha1/fake/fake_mattermost_client.go | Removes fake v1alpha1 typed client. |
| pkg/client/clientset/versioned/typed/mattermost/v1alpha1/fake/fake_clusterinstallation.go | Removes fake v1alpha1 ClusterInstallation client. |
| pkg/client/clientset/versioned/typed/mattermost/v1alpha1/fake/doc.go | Removes fake typed client package doc. |
| pkg/client/clientset/versioned/typed/mattermost/v1alpha1/doc.go | Removes typed client package doc. |
| pkg/client/clientset/versioned/typed/mattermost/v1alpha1/clusterinstallation.go | Removes typed ClusterInstallation client. |
| pkg/client/clientset/versioned/scheme/register.go | Removes clientset scheme registration for v1alpha1. |
| pkg/client/clientset/versioned/scheme/doc.go | Removes clientset scheme doc. |
| pkg/client/clientset/versioned/fake/register.go | Removes fake clientset scheme registration. |
| pkg/client/clientset/versioned/fake/doc.go | Removes fake clientset doc. |
| pkg/client/clientset/versioned/fake/clientset_generated.go | Removes fake clientset wiring for v1alpha1. |
| pkg/client/clientset/versioned/doc.go | Removes versioned clientset doc. |
| pkg/client/clientset/versioned/clientset.go | Removes versioned clientset implementation (v1alpha1). |
| Makefile | Adds CRD truncation option; removes mysql/minio operator targets and v1alpha1 codegen steps. |
| main.go | Removes ClusterInstallation and MattermostRestoreDB controllers and external-operator schemes. |
| go.mod | Drops MinIO operator dependency, updates k8s/controller-runtime/testify and related indirect deps. |
| docs/minio-operator/minio-operator.yaml | Removes bundled MinIO operator manifest. |
| docs/migration.md | Adds explicit note that migration must be done on v1.24.x or earlier. |
| docs/mattermost_v1beta1_crd.md | Removes operatorManaged DB/fileStore sections from generated CRD docs. |
| docs/examples/simple_aws_azure.yaml | Removes v1alpha1 ClusterInstallation example. |
| docs/examples/simple_anywhere.yaml | Removes v1alpha1 ClusterInstallation example. |
| docs/examples/mattermost_simple_aws_azure.yaml | Updates example to include required external DB + external file store config. |
| docs/examples/mattermost_simple_anywhere.yaml | Updates example to include required external DB + local file store config. |
| docs/examples/mattermost_mysql_minio_operators_small.yaml | Removes example relying on operator-managed MySQL/MinIO. |
| docs/examples/mattermost_full.yaml | Updates size comment to reflect app-only sizing in v2. |
| docs/examples/mattermost_full_aws_alb.yaml | Updates size comment to reflect app-only sizing in v2. |
| docs/examples/full.yaml | Removes v1alpha1 ClusterInstallation full example. |
| controllers/mattermost/mattermostrestoredb/utils.go | Removes MattermostRestoreDB controller utilities. |
| controllers/mattermost/mattermostrestoredb/mysql.go | Removes MattermostRestoreDB MySQL secret updater. |
| controllers/mattermost/mattermostrestoredb/controller.go | Removes MattermostRestoreDB controller implementation. |
| controllers/mattermost/mattermost/mattermost_test.go | Updates tests to assume external DB + external file store are required. |
| controllers/mattermost/mattermost/file_store.go | Removes operator-managed MinIO reconciliation and makes “no filestore” an error path. |
| controllers/mattermost/mattermost/database.go | Removes operator-managed DB reconciliation and makes “no database” an error path. |
| controllers/mattermost/mattermost/controller_test.go | Updates reconcile tests to include external DB/file store and removes MySQL/MinIO assertions. |
| controllers/mattermost/clusterinstallation/utils.go | Removes ClusterInstallation controller utilities and health checks. |
| controllers/mattermost/clusterinstallation/utils_test.go | Removes ClusterInstallation label helper tests. |
| controllers/mattermost/clusterinstallation/postgresql.go | Removes placeholder ClusterInstallation postgres check. |
| controllers/mattermost/clusterinstallation/mysql.go | Removes ClusterInstallation MySQL operator reconciliation logic. |
| controllers/mattermost/clusterinstallation/minio.go | Removes ClusterInstallation MinIO operator reconciliation logic. |
| controllers/mattermost/clusterinstallation/conversion.go | Removes ClusterInstallation→Mattermost conversion/migration logic. |
| controllers/mattermost/clusterinstallation/canary.go | Removes ClusterInstallation canary logic. |
| controllers/mattermost/clusterinstallation/bluegreen.go | Removes ClusterInstallation blue/green logic. |
| config/samples/mattermost.com_v1alpha1_mattermostrestoredb.yaml | Removes v1alpha1 MattermostRestoreDB sample. |
| config/samples/mattermost.com_v1alpha1_clusterinstallation.yaml | Removes v1alpha1 ClusterInstallation sample. |
| config/samples/kustomization.yaml | Removes v1alpha1 sample references from kustomize. |
| config/rbac/role.yaml | Drops RBAC permissions for mattermost.com, mysql.presslabs.org, and miniocontroller.min.io groups. |
| config/rbac/mattermostrestoredb_viewer_role.yaml | Removes viewer role for MattermostRestoreDB. |
| config/rbac/mattermostrestoredb_editor_role.yaml | Removes editor role for MattermostRestoreDB. |
| config/rbac/clusterinstallation_viewer_role.yaml | Removes viewer role for ClusterInstallation. |
| config/rbac/clusterinstallation_editor_role.yaml | Removes editor role for ClusterInstallation. |
| config/crd/kustomization.yaml | Removes v1alpha1 CRDs from CRD kustomization. |
| config/crd/bases/mattermost.com_mattermostrestoredbs.yaml | Removes MattermostRestoreDB CRD. |
| apis/mattermost/v1beta1/zz_generated.deepcopy.go | Updates deepcopy generation to remove operatorManaged fields and add size-related deep copies. |
| apis/mattermost/v1beta1/mattermost_utils.go | Makes FileStore/Database SetDefaults return errors and plumbs them through Mattermost.SetDefaults. |
| apis/mattermost/v1beta1/mattermost_utils_test.go | Updates defaults tests to reflect mandatory DB/file store configuration. |
| apis/mattermost/v1beta1/mattermost_types.go | Removes operatorManaged DB/file store API fields and types. |
| apis/mattermost/v1beta1/mattermost_test.go | Updates sizing tests to app-only presets (no DB/file store sizing). |
| apis/mattermost/v1beta1/mattermost_sizes.go | Moves sizing to v1beta1 and fixes preset aliasing via deep copies. |
| apis/mattermost/v1beta1/mattermost_sizes_test.go | Adds regression tests ensuring size presets aren’t shared/aliased. |
| apis/mattermost/v1beta1/file_store_util.go | Removes operatorManaged defaults; enforces explicit file store selection. |
| apis/mattermost/v1beta1/db_util.go | Removes operatorManaged defaults; enforces explicit external DB selection. |
| apis/mattermost/v1alpha1/mattermostrestoredb_types.go | Removes v1alpha1 MattermostRestoreDB API types. |
| apis/mattermost/v1alpha1/groupversion_info.go | Removes v1alpha1 groupversion/scheme registration. |
| apis/mattermost/v1alpha1/doc.go | Removes v1alpha1 API package doc. |
| .github/workflows/ci.yml | Removes v1alpha1 generate/diff check steps. |
Files not reviewed (1)
- apis/mattermost/v1beta1/zz_generated.deepcopy.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
prepareSchema asserted the same err value twice in a row, a leftover from removing the MinIO and MySQL scheme registrations. Drop the redundant assertion.
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
Comment |
fmartingr
left a comment
There was a problem hiding this comment.
Outstanding work @andrleite ! 🎊
I would give it a pass with an agent to review all doc strings around, it seems there are some mentions to removed fields here and there and there are many mentions on how "things used to be" which are not useful moving on.
Some questions:
- Shouldn't we drop support for MySQL entirely?
- Why keeping the APIs in
v1beta? Are there plans to move them tov1(orv2) at some point?
I've left some comments around in the changes, most of them are just small things that I leave to your judgement. The most critical for me is the code path SetDefault take if we are now using it for validation. I would rename the method name and make a pass to check if there's any problems with that.
Again, great job here!
nickmisasi
left a comment
There was a problem hiding this comment.
Overall LGTM - flagged a couple things in addition to Felipe
| // A database and a file store are now required and the Operator does not | ||
| // provision either, so the suite has to create them before any Mattermost. | ||
| cleanupPrereqs, err := SetupMattermostPrerequisites(context.TODO(), k8sClient, mmNamespace) | ||
| require.NoError(t, err) | ||
| defer cleanupPrereqs() | ||
|
|
There was a problem hiding this comment.
I think this is a slight behaviour regression because prior to this change each test would have had its own db and minio, but now the scale and upgrade tests share
| if err := mm.Spec.FileStore.SetDefaults(); err != nil { | ||
| return err | ||
| } | ||
| if err := mm.Spec.Database.SetDefaults(); err != nil { | ||
| return err | ||
| } | ||
|
|
There was a problem hiding this comment.
Because SetDefaults now just returns an error, an upgraded operator-managed install will only show an error for the filestore. The tests pass an empty spec and assert only the error for the filestore missing. Not sure we need to have a single error when both are missing (though it would be nice if we didn’t have to have 2 failures) but we should at least add test coverage for the case of filestore existing but no database
…e comments - Rename Database.SetDefaults() and FileStore.SetDefaults() to IsValid() since they validate configuration rather than set defaults - Collect both filestore and database validation errors before returning, so a misconfigured install surfaces all problems in one reconcile loop - Add missing test cases: filestore-present-but-no-database, and both-missing returning a combined error message - Remove stale comments that referenced how the operator used to work Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move SetupMattermostPrerequisites inside each sub-test so every test gets a fresh Postgres and MinIO and cleans up after itself. Tests run sequentially (no t.Parallel), so the first test's defer fires before the second test starts — no resource-name conflicts. Previously each Mattermost CR got its own operator-managed MySQL, so isolation was automatic. This restores the equivalent behaviour for the external-DB path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…estore DefaultMattermostDatabaseType, DefaultStorageSize, DefaultDatabaseVersion and DefaultMattermostSize had no usages outside their own declarations after the operator-managed MySQL and MinIO paths were removed in v2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ClusterInstallation no longer exists so the "Cluster" prefix is misleading. GetMattermostSize better reflects what the function does: look up a size preset for a Mattermost resource.
- Update README to note PostgreSQL is the only supported database going forward (Mattermost Server v10+ deprecates MySQL) and add a callout recommending migration for existing MySQL users - Fix misleading "MySQL no longer exists" comments in tests — external MySQL connections still work; only operator-managed MySQL was removed Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Thanks @fmartingr . Drop support for MySQL entirely, agreed. Mattermost officially dropped MySQL for new installs in v10, with full end-of-support in v11. We removed operator-managed MySQL in this PR and updated the README to recommend PostgreSQL. The external MySQL code path stays for existing users mid-migration, but a follow-up PR should add a validation warning when a MySQL connection string is detected so the deprecation is explicit. Regarding |
Go 1.26 extended new() to accept value expressions; new(DefaultSize.App.Replicas) allocates a fresh int32 initialised from the preset value — a copy, not an alias — making the intent clearer than utils.NewInt32. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Operator v2.0.0. Removes the long-deprecated Custom Resources and both "delegate to another operator" integrations, and brings the Kubernetes libraries up to date.
127 files changed, 1844 insertions(+), 27169 deletions(-)Removed
ClusterInstallation(mattermost.com/v1alpha1)Mattermost. Carried BlueGreen/Canary, a MySQL and a PostgreSQL path, MinIO provisioning, and the CI→MM conversion code.MattermostRestoreDB(mattermost.com/v1alpha1)ClusterInstallation.spec.fileStore.operatorManagedMinIOInstancefor the archivedminio/minio-operator. See below.spec.database.operatorManagedMysqlClusterfor the presslabs MySQL operator, whose API types were vendored into this repo.apis/mattermost/v1alpha1Result: the Operator now depends on no third-party operator. Its ClusterRole requests permissions only on
"",coordination.k8s.io,apps,batch,networking.k8s.io,rbac.authorization.k8s.io, andinstallation.mattermost.com— bothminiocontroller.min.ioandmysql.presslabs.orgare gone.Dependency upgrade (first commit, non-breaking and reviewable on its own):
k8s.io/*v0.33.1 → v0.36.3,controller-runtimev0.21.0 → v0.24.1, addingstructured-merge-diff/v6. No source changes were needed. Rebased onto current master, so Go 1.26.6 and master'sgolang.org/x/*bumps (incl.x/netv0.58.0) are preserved — all newer than what the k8s bump pulled in.Why MinIO had to go, specifically:
github.com/minio/minio-operatorwas last published in February 2020 and is archived. Theminiocontroller.min.io/v1beta1 MinIOInstanceAPI isn't served by any current MinIO operator (which moved tominio.min.io/v2 Tenant), so the path has been incompatible with a supported MinIO operator for years. Worse, theMinIOInstancewas created without an image, so the MinIO server holding all user uploads ran whateverminio/k8s-operator:1.0.7defaulted to — invisible to image scanning, because no tag appeared anywhere in this repo. Removal also drops two init containers from every pod in that mode, one of which ranappropriate/curl:latest(abandoned image, mutable tag).Commits
The removal is structured so each step can be reviewed and reverted independently:
f1ec29d2Update k8s libraries to v0.36.3 and controller-runtime to v0.24.1b1e32c37Remove the MattermostRestoreDB custom resource007cd488Remove the ClusterInstallation custom resource and controller7455b02cRemove operator-managed MinIO file storeb6ff2036Remove operator-managed MySQL databasee1a7c988Remove the v1alpha1 API package and move size presets to v1beta1d2e20ecdRelease Operator v2.0.0Follow-ups from review and CI:
6a40ac16fix: apply CodeRabbit auto-fixesd794a55cFix shared size presets and give the e2e suite its prerequisites2a65c3f2Use the external MinIO fixture for the e2e file storee83683fdReconcile the CodeRabbit auto-fix commite638f379Assert replica isolation in the explicit-size test836c7c3aUpdate CI for the removed v1alpha1 packageb7754489Wait for Postgres and MinIO before creating Mattermost resources in e2e0891657cKeep the generated CRD under the annotation size limitReviewer notes
A database and file store are now mandatory — deliberately with no replacement default. Until now a
Mattermostwith neither section silently received in-cluster MinIO and MySQL; such a resource is now rejected with an explicit error. Defaulting tolocalinstead would have repointed an installation at an empty PVC while its files stayed in the old MinIO — data loss from the user's perspective. Failing loudly forces a conscious choice and a migration. Two example manifests relied on that silent default and have been fixed.Nothing deletes user data. No code path removes a
MinIOInstanceorMysqlCluster. They are left running but unmanaged, so operators can migrate at their own pace. Confirmed on a real cluster — see Testing.Size presets shrank but behave identically.
ClusterInstallationSize{App, Minio, Database}→v1beta1.Size{App}, since the other two only sized the removed workloads. All nine size keys keep their existing app-server replicas and resource requests, sospec.sizeis unchanged.Correctness fix worth a look (
d794a55c). The size presets were aliased rather than copied:setDefaultReplicasAndResourcestook&DefaultSize.App.Replicas, handing everyMattermosta pointer into a package-level value, andResourceRequirementsassignment shares itsResourceListmaps by reference. Scaling one installation therefore rewrote the preset and changed the defaults every installation reconciled afterwards received. Demonstrated before fixing — one CR movedDefaultSize.App.Replicasfrom 2 to 999 and CPU from 500m to 777m, and a second unrelated CR then inherited those. Two regression tests fail against the previous code. This predates v2 (v1alpha1 aliased identically), but the code moved to v1beta1 here.The 1480-line CRD diff in
0891657cis intentional.make deploybegan failing withmetadata.annotations: Too long: must have at most 262144 bytes.kubectl applystores the manifest inlast-applied-configuration, and measured as JSON — which is what the annotation holds — master sat at 260,965 bytes, 1,179 bytes under the ceiling. The k8s v0.36 bump pushed it over by inlining newcorev1fields; the removals recovered 6.3KB but not enough.CRD_OPTIONSnow passescrd:maxDescLen=200, bringing it to 192,749 bytes, 69KB clear. Consequence:kubectl explainshows truncated descriptions for long fields; the full text remains indocs/mattermost_v1beta1_crd.md, which is generated from Go source and unaffected. Note master is still 1,179 bytes from this same failure and should get the same treatment independently.mattermost-cloud, which imports these packages directly:apis/mattermost/v1alpha1is gone; size presets moved tov1beta1.Size;CalculateResourceMilliRequirements/CalculateCPUMilliRequirement/CalculateMemoryMilliRequirementlost theirincludeDatabase/includeMinioparameters; andOperatorManagedMinio,OperatorManagedDatabase,pkg/components/{minio,mysql}andpkg/mattermost.NewOperatorManagedFileStoreInfoare removed. This needs a coordinated change there and is the remaining blocker.Not in this PR, by design: the health-checker label-selector bug (
MattermostPodLabelsvsMattermostLabels, dates to 2022, breaks any install usingpodTemplate.extraLabels);useServiceLoadBalancerorphaning a leftover Ingress; and bumping the e2eK8S_VERSION(see Testing). All pre-date or sit outside this work and deserve their own PRs.Testing
All CI checks pass. Note
ci/test-e2eis a step inside thetestjob rather than its own check — it stands up a kind cluster, runsmake deploy, then runs both./test/e2eand./test/e2e-external.Validated manually in a real dev cluster:
This matters because the e2e job is pinned to
K8S_VERSION: v1.22.9, 14 minor versions behind the v0.36.3 client libraries this PR moves to — well outside client-go's supported skew. A green e2e is therefore weaker evidence than it appears, and the dev-cluster run is what actually covers modern Kubernetes. Bumping that pin is worth a follow-up, kept out of this PR to avoid surfacing unrelated failures in an already-large change.Locally:
go build ./...,gofmt,go vet ./...(including both e2e packages) andmake unittestpass after every individual commit. Generated artefacts verified idempotent — re-runningmake generate manifestsandkustomize build config/defaultproduces no diff, re-checked after the rebase onto master.The e2e suite was reworked as part of this. It previously got its database from the MySQL operator; it now uses an in-cluster Postgres and a standalone MinIO (as an S3 endpoint) from
resources/, which is whattest/e2e-externalalready did. A newSetupMattermostPrerequisitesapplies both and waits for their Deployments to become available before anyMattermostis created — necessary becausemm-secrets.yamlcarries noDB_CONNECTION_CHECK_URL, so the Operator injects no database readiness init container and nothing else blocks on the database being reachable.Ticket Link
SEC-11130
Release Note