From 110604f1e53ccc230e3b8bfaea6555bb8eab6efd Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 5 Jan 2022 14:12:34 +0000 Subject: [PATCH 01/17] Add 'cmctl upgrade migrate' tool to assist in v1.7 upgrade Signed-off-by: James Munnelly --- cmd/ctl/BUILD.bazel | 1 + cmd/ctl/pkg/build/commands/BUILD.bazel | 1 + cmd/ctl/pkg/build/commands/commands.go | 2 + cmd/ctl/pkg/upgrade/BUILD.bazel | 30 +++ cmd/ctl/pkg/upgrade/migrate/BUILD.bazel | 40 ++++ cmd/ctl/pkg/upgrade/migrate/migrate.go | 280 ++++++++++++++++++++++++ cmd/ctl/pkg/upgrade/upgrade.go | 38 ++++ 7 files changed, 392 insertions(+) create mode 100644 cmd/ctl/pkg/upgrade/BUILD.bazel create mode 100644 cmd/ctl/pkg/upgrade/migrate/BUILD.bazel create mode 100644 cmd/ctl/pkg/upgrade/migrate/migrate.go create mode 100644 cmd/ctl/pkg/upgrade/upgrade.go diff --git a/cmd/ctl/BUILD.bazel b/cmd/ctl/BUILD.bazel index 01d55a0d5..373b66152 100644 --- a/cmd/ctl/BUILD.bazel +++ b/cmd/ctl/BUILD.bazel @@ -61,6 +61,7 @@ filegroup( "//cmd/ctl/pkg/install:all-srcs", "//cmd/ctl/pkg/renew:all-srcs", "//cmd/ctl/pkg/status:all-srcs", + "//cmd/ctl/pkg/upgrade:all-srcs", "//cmd/ctl/pkg/version:all-srcs", ], tags = ["automanaged"], diff --git a/cmd/ctl/pkg/build/commands/BUILD.bazel b/cmd/ctl/pkg/build/commands/BUILD.bazel index b22d8ea9a..ebea90219 100644 --- a/cmd/ctl/pkg/build/commands/BUILD.bazel +++ b/cmd/ctl/pkg/build/commands/BUILD.bazel @@ -16,6 +16,7 @@ go_library( "//cmd/ctl/pkg/inspect:go_default_library", "//cmd/ctl/pkg/renew:go_default_library", "//cmd/ctl/pkg/status:go_default_library", + "//cmd/ctl/pkg/upgrade:go_default_library", "//cmd/ctl/pkg/version:go_default_library", "@com_github_spf13_cobra//:go_default_library", "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", diff --git a/cmd/ctl/pkg/build/commands/commands.go b/cmd/ctl/pkg/build/commands/commands.go index 3319c6046..bd723c995 100644 --- a/cmd/ctl/pkg/build/commands/commands.go +++ b/cmd/ctl/pkg/build/commands/commands.go @@ -33,6 +33,7 @@ import ( "github.com/jetstack/cert-manager/cmd/ctl/pkg/inspect" "github.com/jetstack/cert-manager/cmd/ctl/pkg/renew" "github.com/jetstack/cert-manager/cmd/ctl/pkg/status" + "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade" "github.com/jetstack/cert-manager/cmd/ctl/pkg/version" ) @@ -56,6 +57,7 @@ func Commands() []RegisterCommandFunc { approve.NewCmdApprove, deny.NewCmdDeny, check.NewCmdCheck, + upgrade.NewCmdUpgrade, // Experimental features experimental.NewCmdExperimental, diff --git a/cmd/ctl/pkg/upgrade/BUILD.bazel b/cmd/ctl/pkg/upgrade/BUILD.bazel new file mode 100644 index 000000000..765498195 --- /dev/null +++ b/cmd/ctl/pkg/upgrade/BUILD.bazel @@ -0,0 +1,30 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["upgrade.go"], + importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade", + visibility = ["//visibility:public"], + deps = [ + "//cmd/ctl/pkg/upgrade/migrate:go_default_library", + "@com_github_spf13_cobra//:go_default_library", + "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [ + ":package-srcs", + "//cmd/ctl/pkg/upgrade/migrate:all-srcs", + ], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel new file mode 100644 index 000000000..98e21a178 --- /dev/null +++ b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel @@ -0,0 +1,40 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library") + +go_library( + name = "go_default_library", + srcs = ["migrate.go"], + importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrate", + visibility = ["//visibility:public"], + deps = [ + "//cmd/ctl/pkg/build:go_default_library", + "//cmd/ctl/pkg/factory:go_default_library", + "//internal/apis/acme/install:go_default_library", + "//internal/apis/certmanager/install:go_default_library", + "@com_github_spf13_cobra//:go_default_library", + "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/install:go_default_library", + "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1:go_default_library", + "@io_k8s_apimachinery//pkg/api/errors:go_default_library", + "@io_k8s_apimachinery//pkg/apis/meta/v1/unstructured:go_default_library", + "@io_k8s_apimachinery//pkg/runtime:go_default_library", + "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", + "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", + "@io_k8s_kubectl//pkg/cmd/util:go_default_library", + "@io_k8s_kubectl//pkg/util/i18n:go_default_library", + "@io_k8s_kubectl//pkg/util/templates:go_default_library", + "@io_k8s_sigs_controller_runtime//pkg/client:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/cmd/ctl/pkg/upgrade/migrate/migrate.go b/cmd/ctl/pkg/upgrade/migrate/migrate.go new file mode 100644 index 000000000..a9f913bc7 --- /dev/null +++ b/cmd/ctl/pkg/upgrade/migrate/migrate.go @@ -0,0 +1,280 @@ +/* +Copyright 2022 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrate + +import ( + "context" + "fmt" + + "github.com/spf13/cobra" + apiextinstall "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" + apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/cli-runtime/pkg/genericclioptions" + cmdutil "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/kubectl/pkg/util/i18n" + "k8s.io/kubectl/pkg/util/templates" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/jetstack/cert-manager/cmd/ctl/pkg/build" + "github.com/jetstack/cert-manager/cmd/ctl/pkg/factory" + acmeinstall "github.com/jetstack/cert-manager/internal/apis/acme/install" + cminstall "github.com/jetstack/cert-manager/internal/apis/certmanager/install" +) + +var ( + long = templates.LongDesc(i18n.T(` +Ensures resources in your Kubernetes cluster are persisted in the v1 API version. + +This must be run prior to upgrading to ensure your cluster is ready to upgrade to cert-manager v1.7 and beyond. + +This command must be run with a cluster running cert-manager v1.0 or greater.`)) + + example = templates.Examples(i18n.T(build.WithTemplate(` +# Check the cert-manager installation is ready to be upgraded to v1.7 +{{.BuildName}} upgrade migrate +`))) +) + +var scheme = runtime.NewScheme() + +func init() { + apiextinstall.Install(scheme) + cminstall.Install(scheme) + acmeinstall.Install(scheme) +} + +// Options is a struct to support renew command +type Options struct { + genericclioptions.IOStreams + *factory.Factory + + client client.Client +} + +// NewOptions returns initialized Options +func NewOptions(ioStreams genericclioptions.IOStreams) *Options { + return &Options{ + IOStreams: ioStreams, + } +} + +// NewCmdMigrate returns a cobra command for updating resources in an apiserver +// to force a new storage version to be used. +func NewCmdMigrate(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.Command { + o := NewOptions(ioStreams) + cmd := &cobra.Command{ + Use: "migrate", + Short: "Migrate all existing persisted cert-manager resources to the v1 API version", + Long: long, + Example: example, + Run: func(cmd *cobra.Command, args []string) { + cmdutil.CheckErr(o.Validate(cmd, args)) + cmdutil.CheckErr(o.Complete()) + cmdutil.CheckErr(o.Run(ctx, args)) + }, + } + + o.Factory = factory.New(ctx, cmd) + + return cmd +} + +// Validate validates the provided options +func (o *Options) Validate(cmd *cobra.Command, args []string) error { + return nil +} + +// Complete takes the command arguments and factory and infers any remaining options. +func (o *Options) Complete() error { + var err error + o.client, err = client.New(o.RESTConfig, client.Options{Scheme: scheme}) + if err != nil { + return err + } + + return nil +} + +// Run executes renew command +func (o *Options) Run(ctx context.Context, args []string) error { + // Check all cert-manager CRDs to ensure that they have their + // storage version set to v1. + allCRDNames := []string{ + "certificates.cert-manager.io", + "certificaterequests.cert-manager.io", + "issuers.cert-manager.io", + "clusterissuers.cert-manager.io", + "orders.acme.cert-manager.io", + "challenges.acme.cert-manager.io", + } + + fmt.Fprintln(o.Out, "Checking all cert-manager CustomResourceDefinitions have storage version set to 'v1'") + allV1, crdsRequiringMigration, err := o.ensureCRDStorageVersionEquals(ctx, "v1", allCRDNames) + if err != nil { + return err + } + if !allV1 { + fmt.Fprintln(o.ErrOut, "It looks like you are running a pre-1.0 version of cert-manager. Please upgrade cert-manager to v1.6 before upgrading to v1.7.") + return fmt.Errorf("migration failed") + } + fmt.Fprintln(o.Out, "All CustomResourceDefinitions have 'v1' configured as the storage version.") + + /* + fmt.Fprintln(o.Out, "Looking for CRDs that contain resources that require migrating to 'v1'...") + crdsRequiringMigration, err := o.discoverCRDsRequiringMigration(ctx, "v1", allCRDNames) + if err != nil { + fmt.Fprintf(o.ErrOut, "Failed to determine resource types that require migration: %v\n", err) + return err + } + if len(crdsRequiringMigration) == 0 { + fmt.Fprintln(o.Out, "Nothing to do. cert-manager CRDs do not have 'status.storedVersions' containing old API versions. You may proceed to upgrade to cert-manager v1.7.") + return nil + } + */ + + fmt.Fprintf(o.Out, "Found %d resource types that require migration:\n", len(crdsRequiringMigration)) + for _, crd := range crdsRequiringMigration { + fmt.Fprintf(o.Out, " - %s\n", crd.Name) + } + + for _, crd := range crdsRequiringMigration { + fmt.Fprintf(o.Out, "Migrating %q objects in group %q - this may take a while...\n", crd.Spec.Names.Kind, crd.Spec.Group) + + if err := o.migrateResourcesForCRD(ctx, crd); err != nil { + fmt.Fprintf(o.ErrOut, "Failed to migrate resource: %v\n", err) + return err + } + } + + fmt.Fprintln(o.Out, "Patching CRD resources to set 'status.storedVersions' to 'v1'...") + if err := o.patchCRDStoredVersions(ctx, crdsRequiringMigration); err != nil { + fmt.Fprintf(o.ErrOut, "Failed to patch 'status.storedVersions' field: %v\n", err) + return err + } + + fmt.Fprintln(o.Out, "Successfully migrated all cert-manager resource types. It is now safe to proceed with upgrading to cert-manager v1.7.") + return nil +} + +func (o *Options) ensureCRDStorageVersionEquals(ctx context.Context, vers string, names []string) (bool, []*apiext.CustomResourceDefinition, error) { + var crds []*apiext.CustomResourceDefinition + for _, crdName := range names { + crd := &apiext.CustomResourceDefinition{} + if err := o.client.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { + return false, nil, err + } + + // Discover the storage version + storageVersion := "" + for _, v := range crd.Spec.Versions { + if v.Storage { + storageVersion = v.Name + break + } + } + + if storageVersion != vers { + fmt.Fprintf(o.Out, "CustomResourceDefinition object %q has storage version set to %q. You MUST upgrade to cert-manager v1.0-v1.6 before migrating resources for v1.7.\n", crdName, storageVersion) + return false, nil, nil + } + + crds = append(crds, crd) + } + + return true, crds, nil +} + +func (o *Options) discoverCRDsRequiringMigration(ctx context.Context, desiredStorageVersion string, names []string) ([]*apiext.CustomResourceDefinition, error) { + var requireMigration []*apiext.CustomResourceDefinition + for _, name := range names { + crd := &apiext.CustomResourceDefinition{} + if err := o.client.Get(ctx, client.ObjectKey{Name: name}, crd); err != nil { + return nil, err + } + // If no versions are stored, there's nothing to migrate. + if len(crd.Status.StoredVersions) == 0 { + continue + } + // If more than one entry exists in `storedVersions` OR if the only element in there is not + // the desired version, perform a migration. + if len(crd.Status.StoredVersions) > 1 || crd.Status.StoredVersions[0] != desiredStorageVersion { + requireMigration = append(requireMigration, crd) + } + } + return requireMigration, nil +} + +func (o *Options) migrateResourcesForCRD(ctx context.Context, crd *apiext.CustomResourceDefinition) error { + list := &unstructured.UnstructuredList{} + list.SetGroupVersionKind(schema.GroupVersionKind{ + Group: crd.Spec.Group, + Version: "v1", + Kind: crd.Spec.Names.ListKind, + }) + if err := o.client.List(ctx, list); err != nil { + return err + } + fmt.Fprintf(o.Out, " %d resources to migrate\n", len(list.Items)) + for _, obj := range list.Items { + if err := o.client.Update(ctx, &obj); handleUpdateErr(err) != nil { + return err + } + } + return nil +} + +// handleUpdateErr will absorb certain types of errors that we know can be skipped/passed on +// during a migration of a particular object. +func handleUpdateErr(err error) error { + if err == nil { + return nil + } + // If the resource no longer exists, don't return the error as the object no longer + // needs updating to the new API version. + if apierrors.IsNotFound(err) { + return nil + } + // If there was a conflict, another client must have written the object already which + // means we don't need to force an update. + if apierrors.IsConflict(err) { + return nil + } + return err +} + +func (o *Options) patchCRDStoredVersions(ctx context.Context, crds []*apiext.CustomResourceDefinition) error { + for _, crd := range crds { + // fetch a fresh copy of the CRD to avoid any conflict errors + freshCRD := &apiext.CustomResourceDefinition{} + if err := o.client.Get(ctx, client.ObjectKey{Name: crd.Name}, freshCRD); err != nil { + return err + } + + // Set the `status.storedVersions` field to 'v1' + freshCRD.Status.StoredVersions = []string{"v1"} + + if err := o.client.Status().Update(ctx, freshCRD); err != nil { + return err + } + } + + return nil +} diff --git a/cmd/ctl/pkg/upgrade/upgrade.go b/cmd/ctl/pkg/upgrade/upgrade.go new file mode 100644 index 000000000..88fb98e31 --- /dev/null +++ b/cmd/ctl/pkg/upgrade/upgrade.go @@ -0,0 +1,38 @@ +/* +Copyright 2022 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package upgrade + +import ( + "context" + + "github.com/spf13/cobra" + "k8s.io/cli-runtime/pkg/genericclioptions" + + "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrate" +) + +func NewCmdUpgrade(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.Command { + cmds := &cobra.Command{ + Use: "upgrade", + Short: "Tools that assist in upgrading cert-manager", + Long: `Note: this command does NOT actually upgrade cert-manager installations`, + } + + cmds.AddCommand(migrate.NewCmdMigrate(ctx, ioStreams)) + + return cmds +} From 0ca2db69a3f6c763d677aae33515c6de98467482 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 5 Jan 2022 15:12:43 +0000 Subject: [PATCH 02/17] Print start time and how long it took to migrate resources Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/migrate.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/ctl/pkg/upgrade/migrate/migrate.go b/cmd/ctl/pkg/upgrade/migrate/migrate.go index a9f913bc7..e73ab28fa 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrate.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrate.go @@ -19,6 +19,7 @@ package migrate import ( "context" "fmt" + "time" "github.com/spf13/cobra" apiextinstall "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" @@ -156,8 +157,6 @@ func (o *Options) Run(ctx context.Context, args []string) error { } for _, crd := range crdsRequiringMigration { - fmt.Fprintf(o.Out, "Migrating %q objects in group %q - this may take a while...\n", crd.Spec.Names.Kind, crd.Spec.Group) - if err := o.migrateResourcesForCRD(ctx, crd); err != nil { fmt.Fprintf(o.ErrOut, "Failed to migrate resource: %v\n", err) return err @@ -223,6 +222,8 @@ func (o *Options) discoverCRDsRequiringMigration(ctx context.Context, desiredSto } func (o *Options) migrateResourcesForCRD(ctx context.Context, crd *apiext.CustomResourceDefinition) error { + startTime := time.Now() + fmt.Fprintf(o.Out, "Migrating %q objects in group %q - this may take a while (started at %s)...\n", crd.Spec.Names.Kind, crd.Spec.Group, startTime.Format(time.Stamp)) list := &unstructured.UnstructuredList{} list.SetGroupVersionKind(schema.GroupVersionKind{ Group: crd.Spec.Group, @@ -238,6 +239,7 @@ func (o *Options) migrateResourcesForCRD(ctx context.Context, crd *apiext.Custom return err } } + fmt.Fprintf(o.Out, " Successfully migrated %d %s objects in %s\n", len(list.Items), crd.Spec.Names.Kind, time.Now().Sub(startTime).Round(time.Second)) return nil } From 438d74be152d8e2c481bf4c236da905e1a7cb0cf Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 5 Jan 2022 17:58:49 +0000 Subject: [PATCH 03/17] Reorganise migrate command into a its own struct & add --force flag Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/BUILD.bazel | 5 +- cmd/ctl/pkg/upgrade/migrate/command.go | 123 +++++++++++ cmd/ctl/pkg/upgrade/migrate/migrate.go | 282 ------------------------ cmd/ctl/pkg/upgrade/migrate/migrator.go | 207 +++++++++++++++++ 4 files changed, 334 insertions(+), 283 deletions(-) create mode 100644 cmd/ctl/pkg/upgrade/migrate/command.go delete mode 100644 cmd/ctl/pkg/upgrade/migrate/migrate.go create mode 100644 cmd/ctl/pkg/upgrade/migrate/migrator.go diff --git a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel index 98e21a178..287efe6e6 100644 --- a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel +++ b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel @@ -2,7 +2,10 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "go_default_library", - srcs = ["migrate.go"], + srcs = [ + "command.go", + "migrator.go", + ], importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrate", visibility = ["//visibility:public"], deps = [ diff --git a/cmd/ctl/pkg/upgrade/migrate/command.go b/cmd/ctl/pkg/upgrade/migrate/command.go new file mode 100644 index 000000000..e5a7f5078 --- /dev/null +++ b/cmd/ctl/pkg/upgrade/migrate/command.go @@ -0,0 +1,123 @@ +/* +Copyright 2022 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrate + +import ( + "context" + + "github.com/spf13/cobra" + apiextinstall "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/cli-runtime/pkg/genericclioptions" + cmdutil "k8s.io/kubectl/pkg/cmd/util" + "k8s.io/kubectl/pkg/util/i18n" + "k8s.io/kubectl/pkg/util/templates" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/jetstack/cert-manager/cmd/ctl/pkg/build" + "github.com/jetstack/cert-manager/cmd/ctl/pkg/factory" + acmeinstall "github.com/jetstack/cert-manager/internal/apis/acme/install" + cminstall "github.com/jetstack/cert-manager/internal/apis/certmanager/install" +) + +var ( + long = templates.LongDesc(i18n.T(` +Ensures resources in your Kubernetes cluster are persisted in the v1 API version. + +This must be run prior to upgrading to ensure your cluster is ready to upgrade to cert-manager v1.7 and beyond. + +This command must be run with a cluster running cert-manager v1.0 or greater.`)) + + example = templates.Examples(i18n.T(build.WithTemplate(` +# Check the cert-manager installation is ready to be upgraded to v1.7 +{{.BuildName}} upgrade migrate +`))) +) + +// Options is a struct to support renew command +type Options struct { + genericclioptions.IOStreams + *factory.Factory + + client client.Client + force bool +} + +// NewOptions returns initialized Options +func NewOptions(ioStreams genericclioptions.IOStreams) *Options { + return &Options{ + IOStreams: ioStreams, + } +} + +// NewCmdMigrate returns a cobra command for updating resources in an apiserver +// to force a new storage version to be used. +func NewCmdMigrate(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.Command { + o := NewOptions(ioStreams) + cmd := &cobra.Command{ + Use: "migrate", + Short: "Migrate all existing persisted cert-manager resources to the v1 API version", + Long: long, + Example: example, + Run: func(cmd *cobra.Command, args []string) { + cmdutil.CheckErr(o.Validate(args)) + cmdutil.CheckErr(o.Complete()) + cmdutil.CheckErr(o.Run(ctx, args)) + }, + } + + cmd.Flags().BoolVarP(&o.force, "force", "f", o.force, ""+ + "If true, all resources will be read and written regardless of the 'status.storedVersions' on the CRD resource. "+ + "Use this mode if you have previously manually modified the 'status.storedVersions' field on CRD resources.") + + o.Factory = factory.New(ctx, cmd) + + return cmd +} + +// Validate validates the provided options +func (o *Options) Validate(_ []string) error { + return nil +} + +// Complete takes the command arguments and factory and infers any remaining options. +func (o *Options) Complete() error { + var err error + scheme := runtime.NewScheme() + apiextinstall.Install(scheme) + cminstall.Install(scheme) + acmeinstall.Install(scheme) + + o.client, err = client.New(o.RESTConfig, client.Options{Scheme: scheme}) + if err != nil { + return err + } + + return nil +} + +// Run executes renew command +func (o *Options) Run(ctx context.Context, args []string) error { + return NewMigrator(o.client, o.force, o.Out, o.ErrOut).Run(ctx, "v1", []string{ + "certificates.cert-manager.io", + "certificaterequests.cert-manager.io", + "issuers.cert-manager.io", + "clusterissuers.cert-manager.io", + "orders.acme.cert-manager.io", + "challenges.acme.cert-manager.io", + }) +} diff --git a/cmd/ctl/pkg/upgrade/migrate/migrate.go b/cmd/ctl/pkg/upgrade/migrate/migrate.go deleted file mode 100644 index e73ab28fa..000000000 --- a/cmd/ctl/pkg/upgrade/migrate/migrate.go +++ /dev/null @@ -1,282 +0,0 @@ -/* -Copyright 2022 The cert-manager Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package migrate - -import ( - "context" - "fmt" - "time" - - "github.com/spf13/cobra" - apiextinstall "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" - apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/cli-runtime/pkg/genericclioptions" - cmdutil "k8s.io/kubectl/pkg/cmd/util" - "k8s.io/kubectl/pkg/util/i18n" - "k8s.io/kubectl/pkg/util/templates" - "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/jetstack/cert-manager/cmd/ctl/pkg/build" - "github.com/jetstack/cert-manager/cmd/ctl/pkg/factory" - acmeinstall "github.com/jetstack/cert-manager/internal/apis/acme/install" - cminstall "github.com/jetstack/cert-manager/internal/apis/certmanager/install" -) - -var ( - long = templates.LongDesc(i18n.T(` -Ensures resources in your Kubernetes cluster are persisted in the v1 API version. - -This must be run prior to upgrading to ensure your cluster is ready to upgrade to cert-manager v1.7 and beyond. - -This command must be run with a cluster running cert-manager v1.0 or greater.`)) - - example = templates.Examples(i18n.T(build.WithTemplate(` -# Check the cert-manager installation is ready to be upgraded to v1.7 -{{.BuildName}} upgrade migrate -`))) -) - -var scheme = runtime.NewScheme() - -func init() { - apiextinstall.Install(scheme) - cminstall.Install(scheme) - acmeinstall.Install(scheme) -} - -// Options is a struct to support renew command -type Options struct { - genericclioptions.IOStreams - *factory.Factory - - client client.Client -} - -// NewOptions returns initialized Options -func NewOptions(ioStreams genericclioptions.IOStreams) *Options { - return &Options{ - IOStreams: ioStreams, - } -} - -// NewCmdMigrate returns a cobra command for updating resources in an apiserver -// to force a new storage version to be used. -func NewCmdMigrate(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.Command { - o := NewOptions(ioStreams) - cmd := &cobra.Command{ - Use: "migrate", - Short: "Migrate all existing persisted cert-manager resources to the v1 API version", - Long: long, - Example: example, - Run: func(cmd *cobra.Command, args []string) { - cmdutil.CheckErr(o.Validate(cmd, args)) - cmdutil.CheckErr(o.Complete()) - cmdutil.CheckErr(o.Run(ctx, args)) - }, - } - - o.Factory = factory.New(ctx, cmd) - - return cmd -} - -// Validate validates the provided options -func (o *Options) Validate(cmd *cobra.Command, args []string) error { - return nil -} - -// Complete takes the command arguments and factory and infers any remaining options. -func (o *Options) Complete() error { - var err error - o.client, err = client.New(o.RESTConfig, client.Options{Scheme: scheme}) - if err != nil { - return err - } - - return nil -} - -// Run executes renew command -func (o *Options) Run(ctx context.Context, args []string) error { - // Check all cert-manager CRDs to ensure that they have their - // storage version set to v1. - allCRDNames := []string{ - "certificates.cert-manager.io", - "certificaterequests.cert-manager.io", - "issuers.cert-manager.io", - "clusterissuers.cert-manager.io", - "orders.acme.cert-manager.io", - "challenges.acme.cert-manager.io", - } - - fmt.Fprintln(o.Out, "Checking all cert-manager CustomResourceDefinitions have storage version set to 'v1'") - allV1, crdsRequiringMigration, err := o.ensureCRDStorageVersionEquals(ctx, "v1", allCRDNames) - if err != nil { - return err - } - if !allV1 { - fmt.Fprintln(o.ErrOut, "It looks like you are running a pre-1.0 version of cert-manager. Please upgrade cert-manager to v1.6 before upgrading to v1.7.") - return fmt.Errorf("migration failed") - } - fmt.Fprintln(o.Out, "All CustomResourceDefinitions have 'v1' configured as the storage version.") - - /* - fmt.Fprintln(o.Out, "Looking for CRDs that contain resources that require migrating to 'v1'...") - crdsRequiringMigration, err := o.discoverCRDsRequiringMigration(ctx, "v1", allCRDNames) - if err != nil { - fmt.Fprintf(o.ErrOut, "Failed to determine resource types that require migration: %v\n", err) - return err - } - if len(crdsRequiringMigration) == 0 { - fmt.Fprintln(o.Out, "Nothing to do. cert-manager CRDs do not have 'status.storedVersions' containing old API versions. You may proceed to upgrade to cert-manager v1.7.") - return nil - } - */ - - fmt.Fprintf(o.Out, "Found %d resource types that require migration:\n", len(crdsRequiringMigration)) - for _, crd := range crdsRequiringMigration { - fmt.Fprintf(o.Out, " - %s\n", crd.Name) - } - - for _, crd := range crdsRequiringMigration { - if err := o.migrateResourcesForCRD(ctx, crd); err != nil { - fmt.Fprintf(o.ErrOut, "Failed to migrate resource: %v\n", err) - return err - } - } - - fmt.Fprintln(o.Out, "Patching CRD resources to set 'status.storedVersions' to 'v1'...") - if err := o.patchCRDStoredVersions(ctx, crdsRequiringMigration); err != nil { - fmt.Fprintf(o.ErrOut, "Failed to patch 'status.storedVersions' field: %v\n", err) - return err - } - - fmt.Fprintln(o.Out, "Successfully migrated all cert-manager resource types. It is now safe to proceed with upgrading to cert-manager v1.7.") - return nil -} - -func (o *Options) ensureCRDStorageVersionEquals(ctx context.Context, vers string, names []string) (bool, []*apiext.CustomResourceDefinition, error) { - var crds []*apiext.CustomResourceDefinition - for _, crdName := range names { - crd := &apiext.CustomResourceDefinition{} - if err := o.client.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { - return false, nil, err - } - - // Discover the storage version - storageVersion := "" - for _, v := range crd.Spec.Versions { - if v.Storage { - storageVersion = v.Name - break - } - } - - if storageVersion != vers { - fmt.Fprintf(o.Out, "CustomResourceDefinition object %q has storage version set to %q. You MUST upgrade to cert-manager v1.0-v1.6 before migrating resources for v1.7.\n", crdName, storageVersion) - return false, nil, nil - } - - crds = append(crds, crd) - } - - return true, crds, nil -} - -func (o *Options) discoverCRDsRequiringMigration(ctx context.Context, desiredStorageVersion string, names []string) ([]*apiext.CustomResourceDefinition, error) { - var requireMigration []*apiext.CustomResourceDefinition - for _, name := range names { - crd := &apiext.CustomResourceDefinition{} - if err := o.client.Get(ctx, client.ObjectKey{Name: name}, crd); err != nil { - return nil, err - } - // If no versions are stored, there's nothing to migrate. - if len(crd.Status.StoredVersions) == 0 { - continue - } - // If more than one entry exists in `storedVersions` OR if the only element in there is not - // the desired version, perform a migration. - if len(crd.Status.StoredVersions) > 1 || crd.Status.StoredVersions[0] != desiredStorageVersion { - requireMigration = append(requireMigration, crd) - } - } - return requireMigration, nil -} - -func (o *Options) migrateResourcesForCRD(ctx context.Context, crd *apiext.CustomResourceDefinition) error { - startTime := time.Now() - fmt.Fprintf(o.Out, "Migrating %q objects in group %q - this may take a while (started at %s)...\n", crd.Spec.Names.Kind, crd.Spec.Group, startTime.Format(time.Stamp)) - list := &unstructured.UnstructuredList{} - list.SetGroupVersionKind(schema.GroupVersionKind{ - Group: crd.Spec.Group, - Version: "v1", - Kind: crd.Spec.Names.ListKind, - }) - if err := o.client.List(ctx, list); err != nil { - return err - } - fmt.Fprintf(o.Out, " %d resources to migrate\n", len(list.Items)) - for _, obj := range list.Items { - if err := o.client.Update(ctx, &obj); handleUpdateErr(err) != nil { - return err - } - } - fmt.Fprintf(o.Out, " Successfully migrated %d %s objects in %s\n", len(list.Items), crd.Spec.Names.Kind, time.Now().Sub(startTime).Round(time.Second)) - return nil -} - -// handleUpdateErr will absorb certain types of errors that we know can be skipped/passed on -// during a migration of a particular object. -func handleUpdateErr(err error) error { - if err == nil { - return nil - } - // If the resource no longer exists, don't return the error as the object no longer - // needs updating to the new API version. - if apierrors.IsNotFound(err) { - return nil - } - // If there was a conflict, another client must have written the object already which - // means we don't need to force an update. - if apierrors.IsConflict(err) { - return nil - } - return err -} - -func (o *Options) patchCRDStoredVersions(ctx context.Context, crds []*apiext.CustomResourceDefinition) error { - for _, crd := range crds { - // fetch a fresh copy of the CRD to avoid any conflict errors - freshCRD := &apiext.CustomResourceDefinition{} - if err := o.client.Get(ctx, client.ObjectKey{Name: crd.Name}, freshCRD); err != nil { - return err - } - - // Set the `status.storedVersions` field to 'v1' - freshCRD.Status.StoredVersions = []string{"v1"} - - if err := o.client.Status().Update(ctx, freshCRD); err != nil { - return err - } - } - - return nil -} diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go new file mode 100644 index 000000000..7d05427c7 --- /dev/null +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -0,0 +1,207 @@ +package migrate + +import ( + "context" + "fmt" + "io" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "time" + + apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +type Migrator struct { + // Client used for API interactions + Client client.Client + + // If true, skip checking the 'status.storedVersion' before running the migration. + // By default, migration will only be run if the CRD contains storedVersions other + // than the desired target version. + Force bool + + // Writers to write informational & error messages to + Out, ErrOut io.Writer +} + +// NewMigrator creates a new migrator with the given API client. +// If either of out or errOut are nil, log messages will be discarded. +func NewMigrator(client client.Client, force bool, out, errOut io.Writer) *Migrator { + if out == nil { + out = io.Discard + } + if errOut == nil { + errOut = io.Discard + } + + return &Migrator{ + Client: client, + Force: force, + Out: out, + ErrOut: errOut, + } +} + +// Run begins the migration of all the named CRDs. +// It will attempt to migrate all resources defined as part of these CRDs to the +// given 'targetVersion', and after completion will update the `status.storedVersions` +// field on the corresponding CRD version to only contain the given targetVersion. +func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string) error { + fmt.Fprintf(m.Out, "Checking all CustomResourceDefinitions have storage version set to '%s'\n", targetVersion) + allV1, allCRDs, err := m.ensureCRDStorageVersionEquals(ctx, targetVersion, names) + if err != nil { + return err + } + if !allV1 { + fmt.Fprintln(m.ErrOut, "It looks like you are running a pre-1.0 version of cert-manager. Please upgrade cert-manager to v1.6 before upgrading to v1.7.") + return fmt.Errorf("preflight checks failed") + } + fmt.Fprintf(m.Out, "All CustomResourceDefinitions have %q configured as the storage version.\n", targetVersion) + + crdsRequiringMigration := allCRDs + if !m.Force { + fmt.Fprintln(m.Out, "Looking for CRDs that contain resources that require migrating to 'v1'...") + crdsRequiringMigration, err = m.discoverCRDsRequiringMigration(ctx, "v1", names) + if err != nil { + fmt.Fprintf(m.ErrOut, "Failed to determine resource types that require migration: %v\n", err) + return err + } + if len(crdsRequiringMigration) == 0 { + fmt.Fprintln(m.Out, "Nothing to do. cert-manager CRDs do not have 'status.storedVersions' containing old API versions. You may proceed to upgrade to cert-manager v1.7.") + return nil + } + } else { + fmt.Fprintln(m.Out, "Forcing migration of all CRD resources as --force=true") + } + + fmt.Fprintf(m.Out, "Found %d resource types that require migration:\n", len(crdsRequiringMigration)) + for _, crd := range crdsRequiringMigration { + fmt.Fprintf(m.Out, " - %s\n", crd.Name) + } + + for _, crd := range crdsRequiringMigration { + if err := m.migrateResourcesForCRD(ctx, crd); err != nil { + fmt.Fprintf(m.ErrOut, "Failed to migrate resource: %v\n", err) + return err + } + } + + fmt.Fprintf(m.Out, "Patching CRD resources to set 'status.storedVersions' to %q...\n", targetVersion) + if err := m.patchCRDStoredVersions(ctx, crdsRequiringMigration); err != nil { + fmt.Fprintf(m.ErrOut, "Failed to patch 'status.storedVersions' field: %v\n", err) + return err + } + + fmt.Fprintln(m.Out, "Successfully migrated all cert-manager resource types. It is now safe to proceed with upgrading to cert-manager v1.7.") + return nil +} + +func (m *Migrator) ensureCRDStorageVersionEquals(ctx context.Context, vers string, names []string) (bool, []*apiext.CustomResourceDefinition, error) { + var crds []*apiext.CustomResourceDefinition + for _, crdName := range names { + crd := &apiext.CustomResourceDefinition{} + if err := m.Client.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { + return false, nil, err + } + + // Discover the storage version + storageVersion := "" + for _, v := range crd.Spec.Versions { + if v.Storage { + storageVersion = v.Name + break + } + } + + if storageVersion != vers { + fmt.Fprintf(m.Out, "CustomResourceDefinition object %q has storage version set to %q. You MUST upgrade to cert-manager v1.0-v1.6 before migrating resources for v1.7.\n", crdName, storageVersion) + return false, nil, nil + } + + crds = append(crds, crd) + } + + return true, crds, nil +} + +func (m *Migrator) discoverCRDsRequiringMigration(ctx context.Context, desiredStorageVersion string, names []string) ([]*apiext.CustomResourceDefinition, error) { + var requireMigration []*apiext.CustomResourceDefinition + for _, name := range names { + crd := &apiext.CustomResourceDefinition{} + if err := m.Client.Get(ctx, client.ObjectKey{Name: name}, crd); err != nil { + return nil, err + } + // If no versions are stored, there's nothing to migrate. + if len(crd.Status.StoredVersions) == 0 { + continue + } + // If more than one entry exists in `storedVersions` OR if the only element in there is not + // the desired version, perform a migration. + if len(crd.Status.StoredVersions) > 1 || crd.Status.StoredVersions[0] != desiredStorageVersion { + requireMigration = append(requireMigration, crd) + } + } + return requireMigration, nil +} + +func (m *Migrator) migrateResourcesForCRD(ctx context.Context, crd *apiext.CustomResourceDefinition) error { + startTime := time.Now() + fmt.Fprintf(m.Out, "Migrating %q objects in group %q - this may take a while (started at %s)...\n", crd.Spec.Names.Kind, crd.Spec.Group, startTime.Format(time.Stamp)) + list := &unstructured.UnstructuredList{} + list.SetGroupVersionKind(schema.GroupVersionKind{ + Group: crd.Spec.Group, + Version: "v1", + Kind: crd.Spec.Names.ListKind, + }) + if err := m.Client.List(ctx, list); err != nil { + return err + } + fmt.Fprintf(m.Out, " %d resources to migrate\n", len(list.Items)) + for _, obj := range list.Items { + if err := m.Client.Update(ctx, &obj); handleUpdateErr(err) != nil { + return err + } + } + fmt.Fprintf(m.Out, " Successfully migrated %d %s objects in %s\n", len(list.Items), crd.Spec.Names.Kind, time.Now().Sub(startTime).Round(time.Second)) + return nil +} + +func (m *Migrator) patchCRDStoredVersions(ctx context.Context, crds []*apiext.CustomResourceDefinition) error { + for _, crd := range crds { + // fetch a fresh copy of the CRD to avoid any conflict errors + freshCRD := &apiext.CustomResourceDefinition{} + if err := m.Client.Get(ctx, client.ObjectKey{Name: crd.Name}, freshCRD); err != nil { + return err + } + + // Set the `status.storedVersions` field to 'v1' + freshCRD.Status.StoredVersions = []string{"v1"} + + if err := m.Client.Status().Update(ctx, freshCRD); err != nil { + return err + } + } + + return nil +} + +// handleUpdateErr will absorb certain types of errors that we know can be skipped/passed on +// during a migration of a particular object. +func handleUpdateErr(err error) error { + if err == nil { + return nil + } + // If the resource no longer exists, don't return the error as the object no longer + // needs updating to the new API version. + if apierrors.IsNotFound(err) { + return nil + } + // If there was a conflict, another client must have written the object already which + // means we don't need to force an update. + if apierrors.IsConflict(err) { + return nil + } + return err +} From eaa95ebf9e8148a50a5e8952b3ee8f27d66a7785 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 5 Jan 2022 17:59:27 +0000 Subject: [PATCH 04/17] Add integration test for 'upgrade migrate' command Signed-off-by: James Munnelly --- test/integration/ctl/BUILD.bazel | 1 + test/integration/ctl/migrate/BUILD.bazel | 39 ++++ .../ctl/migrate/ctl_upgrade_migrate_test.go | 202 ++++++++++++++++++ 3 files changed, 242 insertions(+) create mode 100644 test/integration/ctl/migrate/BUILD.bazel create mode 100644 test/integration/ctl/migrate/ctl_upgrade_migrate_test.go diff --git a/test/integration/ctl/BUILD.bazel b/test/integration/ctl/BUILD.bazel index ced010a46..c0a0d0547 100644 --- a/test/integration/ctl/BUILD.bazel +++ b/test/integration/ctl/BUILD.bazel @@ -49,6 +49,7 @@ filegroup( srcs = [ ":package-srcs", "//test/integration/ctl/install_framework:all-srcs", + "//test/integration/ctl/migrate:all-srcs", ], tags = ["automanaged"], visibility = ["//visibility:public"], diff --git a/test/integration/ctl/migrate/BUILD.bazel b/test/integration/ctl/migrate/BUILD.bazel new file mode 100644 index 000000000..4477be44a --- /dev/null +++ b/test/integration/ctl/migrate/BUILD.bazel @@ -0,0 +1,39 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_test") + +go_test( + name = "go_default_test", + srcs = ["ctl_upgrade_migrate_test.go"], + data = [ + "//pkg/webhook/handlers/testdata/apis/testgroup/crds:all-srcs", + ], + deps = [ + "//cmd/ctl/pkg/upgrade/migrate:go_default_library", + "//pkg/webhook/handlers:go_default_library", + "//pkg/webhook/handlers/testdata/apis/testgroup/install:go_default_library", + "//pkg/webhook/handlers/testdata/apis/testgroup/v1:go_default_library", + "//pkg/webhook/handlers/testdata/apis/testgroup/v2:go_default_library", + "//test/integration/framework:go_default_library", + "@com_github_go_logr_logr//testing:go_default_library", + "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/install:go_default_library", + "@io_k8s_apiextensions_apiserver//pkg/apis/apiextensions/v1:go_default_library", + "@io_k8s_apimachinery//pkg/apis/meta/v1:go_default_library", + "@io_k8s_apimachinery//pkg/apis/meta/v1/unstructured:go_default_library", + "@io_k8s_apimachinery//pkg/runtime:go_default_library", + "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", + "@io_k8s_sigs_controller_runtime//pkg/client:go_default_library", + ], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go new file mode 100644 index 000000000..9b470e129 --- /dev/null +++ b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go @@ -0,0 +1,202 @@ +/* +Copyright 2022 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrate + +import ( + "context" + "os" + "testing" + "time" + + testlogger "github.com/go-logr/logr/testing" + apiextinstall "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/install" + apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrate" + "github.com/jetstack/cert-manager/pkg/webhook/handlers" + "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/install" + "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1" + "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v2" + "github.com/jetstack/cert-manager/test/integration/framework" +) + +var ( + equivalentResources = map[string]client.Object{ + "v1": &v1.TestType{ + ObjectMeta: metav1.ObjectMeta{ + Name: "object", + Namespace: "default", + }, + TestField: "abc", + TestFieldImmutable: "def", + }, + "v2": &v2.TestType{ + ObjectMeta: metav1.ObjectMeta{ + Name: "object", + Namespace: "default", + }, + TestField: "abc", + TestFieldImmutable: "def", + }, + } +) + +func newScheme() *runtime.Scheme { + scheme := runtime.NewScheme() + apiextinstall.Install(scheme) + install.Install(scheme) + return scheme +} + +func TestCtlUpgradeMigrate(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + defer cancel() + + // Create the control plane with the TestType conversion handlers registered + scheme := newScheme() + // name of the testtype CRD resource + crdName := "testtypes.testgroup.testing.cert-manager.io" + restCfg, stop := framework.RunControlPlane(t, context.Background(), + framework.WithCRDDirectory("../../../../pkg/webhook/handlers/testdata/apis/testgroup/crds"), + framework.WithWebhookConversionHandler(handlers.NewSchemeBackedConverter(testlogger.TestLogger{T: t}, scheme))) + defer stop() + + // Ensure the OpenAPI endpoint has been updated with the TestType CRD + framework.WaitForOpenAPIResourcesToBeLoaded(t, ctx, restCfg, schema.GroupVersionKind{ + Group: "testgroup.testing.cert-manager.io", + Version: "v1", + Kind: "TestType", + }) + + // Create an API client + cl, err := client.New(restCfg, client.Options{Scheme: scheme}) + if err != nil { + t.Fatal(err) + } + + // Fetch a copy of the recently created TestType CRD + crd := &apiext.CustomResourceDefinition{} + if err := cl.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { + t.Fatal(err) + } + + // Identify the current storage version and one non-storage version for this CRD. + // We'll be creating objects and then changing the storage version on the CRD to + // the 'nonStorageVersion' and ensuring the migration/upgrade is successful. + storageVersion, nonStorageVersion := versionsForCRD(crd) + if storageVersion == "" || nonStorageVersion == "" { + t.Fatal("this test requires testdata with both a storage and non-storage version set") + } + + // Ensure the original storage version is the only one on the CRD + if len(crd.Status.StoredVersions) != 1 || crd.Status.StoredVersions[0] != storageVersion { + t.Errorf("Expected status.storedVersions to only contain the storage version %q but it was: %v", storageVersion, crd.Status.StoredVersions) + } + + // Create a resource + obj := equivalentResources[storageVersion] + if err := cl.Create(ctx, obj); err != nil { + t.Errorf("Failed to create test resource: %v", err) + } + + // Set the storage version to the 'nonStorageVersion' + setStorageVersion(crd, nonStorageVersion) + if err := cl.Update(ctx, crd); err != nil { + t.Fatalf("Failed to update CRD storage version: %v", err) + } + if len(crd.Status.StoredVersions) != 2 || crd.Status.StoredVersions[0] != storageVersion || crd.Status.StoredVersions[1] != nonStorageVersion { + t.Fatalf("Expected status.storedVersions to contain [%s, %s] but it was: %v", storageVersion, nonStorageVersion, crd.Status.StoredVersions) + } + + // Run the migrator and migrate all objects to the 'nonStorageVersion' (which is now the new storage version) + migrator := migrate.NewMigrator(cl, false, os.Stdout, os.Stderr) + if err := migrator.Run(ctx, nonStorageVersion, []string{crdName}); err != nil { + t.Errorf("migrator failed to run: %v", err) + } + + // Check the status.storedVersions field to ensure it only contains one element + crd = &apiext.CustomResourceDefinition{} + if err := cl.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { + t.Fatal(err) + } + if len(crd.Status.StoredVersions) != 1 || crd.Status.StoredVersions[0] != nonStorageVersion { + t.Fatalf("Expected status.storedVersions to be %q but it was: %v", nonStorageVersion, crd.Status.StoredVersions) + } + + // Remove the previous storage version from the CRD and update it + removeAPIVersion(crd, storageVersion) + if err := cl.Update(ctx, crd); err != nil { + t.Fatalf("Failed to remove old API version: %v", err) + } + + // Attempt to read a resource list in the new API version + objList := &unstructured.UnstructuredList{} + objList.SetGroupVersionKind(schema.GroupVersionKind{ + Group: crd.Spec.Group, + Version: nonStorageVersion, + Kind: crd.Spec.Names.ListKind, + }) + if err := cl.List(ctx, objList); err != nil { + t.Fatalf("Failed to list objects (gvk %v): %v", objList.GroupVersionKind(), err) + } + if len(objList.Items) != 1 { + t.Fatalf("Expected a single TestType resource to exist") + } +} + +func versionsForCRD(crd *apiext.CustomResourceDefinition) (storage, nonstorage string) { + storageVersion := "" + nonStorageVersion := "" + for _, v := range crd.Spec.Versions { + if v.Storage { + storageVersion = v.Name + } else { + nonStorageVersion = v.Name + } + if storageVersion != "" && nonStorageVersion != "" { + break + } + } + + return storageVersion, nonStorageVersion +} + +func setStorageVersion(crd *apiext.CustomResourceDefinition, newStorageVersion string) { + for i, v := range crd.Spec.Versions { + if v.Name == newStorageVersion { + v.Storage = true + } else if v.Storage { + v.Storage = false + } + crd.Spec.Versions[i] = v + } +} + +func removeAPIVersion(crd *apiext.CustomResourceDefinition, version string) { + var newVersions []apiext.CustomResourceDefinitionVersion + for _, v := range crd.Spec.Versions { + if v.Name != version { + newVersions = append(newVersions, v) + } + } + crd.Spec.Versions = newVersions +} From feca88bae53eee7c8923814bc96dfbba7d57c07d Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 13:16:55 +0000 Subject: [PATCH 05/17] Retry failed requests 3 times before giving up Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/BUILD.bazel | 2 ++ cmd/ctl/pkg/upgrade/migrate/migrator.go | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel index 287efe6e6..7b08d0aee 100644 --- a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel +++ b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel @@ -20,7 +20,9 @@ go_library( "@io_k8s_apimachinery//pkg/apis/meta/v1/unstructured:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", + "@io_k8s_apimachinery//pkg/util/wait:go_default_library", "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", + "@io_k8s_client_go//util/retry:go_default_library", "@io_k8s_kubectl//pkg/cmd/util:go_default_library", "@io_k8s_kubectl//pkg/util/i18n:go_default_library", "@io_k8s_kubectl//pkg/util/templates:go_default_library", diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go index 7d05427c7..eb78f66ab 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -4,10 +4,13 @@ import ( "context" "fmt" "io" + "time" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" - "time" + "k8s.io/apimachinery/pkg/util/wait" + "k8s.io/client-go/util/retry" apiext "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -160,7 +163,14 @@ func (m *Migrator) migrateResourcesForCRD(ctx context.Context, crd *apiext.Custo } fmt.Fprintf(m.Out, " %d resources to migrate\n", len(list.Items)) for _, obj := range list.Items { - if err := m.Client.Update(ctx, &obj); handleUpdateErr(err) != nil { + // retry on any kind of error to handle cases where e.g. the network connection to the apiserver fails + if err := retry.OnError(wait.Backoff{ + Duration: time.Second, // wait 1s between attempts + Steps: 3, // allow up to 3 attempts per object + }, func(err error) bool { + // Retry on any errors that are not otherwise skipped/ignored + return handleUpdateErr(err) != nil + }, func() error { return m.Client.Update(ctx, &obj) }); handleUpdateErr(err) != nil { return err } } From 8fa378fe0188bbfcfa7060c12b496dd46cf2b4b1 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 13:17:39 +0000 Subject: [PATCH 06/17] Check to ensure the storage version or storedVersions have not unexpectedly changed before patching storedVersions Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/BUILD.bazel | 1 + cmd/ctl/pkg/upgrade/migrate/migrator.go | 61 ++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel index 7b08d0aee..b0d5f869b 100644 --- a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel +++ b/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel @@ -20,6 +20,7 @@ go_library( "@io_k8s_apimachinery//pkg/apis/meta/v1/unstructured:go_default_library", "@io_k8s_apimachinery//pkg/runtime:go_default_library", "@io_k8s_apimachinery//pkg/runtime/schema:go_default_library", + "@io_k8s_apimachinery//pkg/util/sets:go_default_library", "@io_k8s_apimachinery//pkg/util/wait:go_default_library", "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", "@io_k8s_client_go//util/retry:go_default_library", diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go index eb78f66ab..50042a71c 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -9,6 +9,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/util/retry" @@ -110,13 +111,7 @@ func (m *Migrator) ensureCRDStorageVersionEquals(ctx context.Context, vers strin } // Discover the storage version - storageVersion := "" - for _, v := range crd.Spec.Versions { - if v.Storage { - storageVersion = v.Name - break - } - } + storageVersion := storageVersionForCRD(crd) if storageVersion != vers { fmt.Fprintf(m.Out, "CustomResourceDefinition object %q has storage version set to %q. You MUST upgrade to cert-manager v1.0-v1.6 before migrating resources for v1.7.\n", crdName, storageVersion) @@ -178,6 +173,9 @@ func (m *Migrator) migrateResourcesForCRD(ctx context.Context, crd *apiext.Custo return nil } +// patchCRDStoredVersions will patch the `status.storedVersions` field of all passed in CRDs to be +// set to an array containing JUST the current storage version. +// This is only safe to run after a successful migration (i.e. a read/write of all resources of the given CRD type). func (m *Migrator) patchCRDStoredVersions(ctx context.Context, crds []*apiext.CustomResourceDefinition) error { for _, crd := range crds { // fetch a fresh copy of the CRD to avoid any conflict errors @@ -186,6 +184,20 @@ func (m *Migrator) patchCRDStoredVersions(ctx context.Context, crds []*apiext.Cu return err } + // Check the latest copy of the CRD to ensure that: + // 1) the storage version is the same as it was at the start of the migration + // 2) the status.storedVersion field has not changed, and if it has, it has only added the new/desired storage version + // This helps to avoid cases where the storage version was changed by a third-party midway through the migration, + // which could lead to corrupted apiservers when we patch the status.storedVersions field below. + expectedStorageVersion := storageVersionForCRD(crd) + if storageVersionForCRD(freshCRD) != expectedStorageVersion { + return newUnexpectedChangeError(crd) + } + newlyAddedVersions := storedVersionsAdded(crd, freshCRD) + if newlyAddedVersions.Len() != 0 || !newlyAddedVersions.Equal(sets.NewString(expectedStorageVersion)) { + return newUnexpectedChangeError(crd) + } + // Set the `status.storedVersions` field to 'v1' freshCRD.Status.StoredVersions = []string{"v1"} @@ -197,6 +209,41 @@ func (m *Migrator) patchCRDStoredVersions(ctx context.Context, crds []*apiext.Cu return nil } +// storageVersionForCRD discovers the storage version for a given CRD. +func storageVersionForCRD(crd *apiext.CustomResourceDefinition) string { + storageVersion := "" + for _, v := range crd.Spec.Versions { + if v.Storage { + storageVersion = v.Name + break + } + } + return storageVersion +} + +// storedVersionsAdded returns a list of any versions added to the `status.storedVersions` field on +// a CRD resource. +func storedVersionsAdded(old, new *apiext.CustomResourceDefinition) sets.String { + oldStoredVersions := sets.NewString(old.Status.StoredVersions...) + newStoredVersions := sets.NewString(new.Status.StoredVersions...) + return newStoredVersions.Difference(oldStoredVersions) +} + +// newUnexpectedChangeError creates a new 'error' that informs users that a change to the CRDs +// was detected during the migration process and so the migration must be re-run. +func newUnexpectedChangeError(crd *apiext.CustomResourceDefinition) error { + errorFmt := "" + + "The CRD %q unexpectedly changed during the migration. " + + "This means that either an object was persisted in a non-storage version during the migration, " + + "or the storage version was changed by someone else (or some automated deployment tooling) whilst the migration " + + "was in progress.\n\n" + + "All automated deployment tooling should be in a 'stable state' (i.e. no upgrades to cert-manager CRDs should be" + + "in progress whilst the migration is running).\n\n" + + "Please ensure no changes to the CRDs are made during the migration process and re-run the migration until you" + + "no longer see this message." + return fmt.Errorf(errorFmt, crd.Name) +} + // handleUpdateErr will absorb certain types of errors that we know can be skipped/passed on // during a migration of a particular object. func handleUpdateErr(err error) error { From b7b1baf56583ec9a96a7a818c39f996942c7c8c7 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 13:18:57 +0000 Subject: [PATCH 07/17] Fix call to NewTestLogger after logr upgrade Signed-off-by: James Munnelly --- test/integration/ctl/migrate/ctl_upgrade_migrate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go index 9b470e129..b9f553357 100644 --- a/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go +++ b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go @@ -77,7 +77,7 @@ func TestCtlUpgradeMigrate(t *testing.T) { crdName := "testtypes.testgroup.testing.cert-manager.io" restCfg, stop := framework.RunControlPlane(t, context.Background(), framework.WithCRDDirectory("../../../../pkg/webhook/handlers/testdata/apis/testgroup/crds"), - framework.WithWebhookConversionHandler(handlers.NewSchemeBackedConverter(testlogger.TestLogger{T: t}, scheme))) + framework.WithWebhookConversionHandler(handlers.NewSchemeBackedConverter(testlogger.NewTestLogger(t), scheme))) defer stop() // Ensure the OpenAPI endpoint has been updated with the TestType CRD From 5b50412d1e9123e2ffb95d2b2ae65305de10ace8 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 14:07:58 +0000 Subject: [PATCH 08/17] Fix bug causing error to always be returned Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/migrator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go index 50042a71c..12d33628a 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -194,7 +194,7 @@ func (m *Migrator) patchCRDStoredVersions(ctx context.Context, crds []*apiext.Cu return newUnexpectedChangeError(crd) } newlyAddedVersions := storedVersionsAdded(crd, freshCRD) - if newlyAddedVersions.Len() != 0 || !newlyAddedVersions.Equal(sets.NewString(expectedStorageVersion)) { + if newlyAddedVersions.Len() != 0 && !newlyAddedVersions.Equal(sets.NewString(expectedStorageVersion)) { return newUnexpectedChangeError(crd) } From f6e34588370e9485b5e2acb4e92d2e0fa1bc35b4 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 15:05:40 +0000 Subject: [PATCH 09/17] Rename --force to --skip-stored-version-check Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/command.go | 9 +++++---- cmd/ctl/pkg/upgrade/migrate/migrator.go | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/cmd/ctl/pkg/upgrade/migrate/command.go b/cmd/ctl/pkg/upgrade/migrate/command.go index e5a7f5078..8d9b1baa3 100644 --- a/cmd/ctl/pkg/upgrade/migrate/command.go +++ b/cmd/ctl/pkg/upgrade/migrate/command.go @@ -53,8 +53,8 @@ type Options struct { genericclioptions.IOStreams *factory.Factory - client client.Client - force bool + client client.Client + skipStoredVersionCheck bool } // NewOptions returns initialized Options @@ -80,7 +80,7 @@ func NewCmdMigrate(ctx context.Context, ioStreams genericclioptions.IOStreams) * }, } - cmd.Flags().BoolVarP(&o.force, "force", "f", o.force, ""+ + cmd.Flags().BoolVar(&o.skipStoredVersionCheck, "skip-stored-version-check", o.skipStoredVersionCheck, ""+ "If true, all resources will be read and written regardless of the 'status.storedVersions' on the CRD resource. "+ "Use this mode if you have previously manually modified the 'status.storedVersions' field on CRD resources.") @@ -112,7 +112,7 @@ func (o *Options) Complete() error { // Run executes renew command func (o *Options) Run(ctx context.Context, args []string) error { - return NewMigrator(o.client, o.force, o.Out, o.ErrOut).Run(ctx, "v1", []string{ + _, err := NewMigrator(o.client, o.skipStoredVersionCheck, o.Out, o.ErrOut).Run(ctx, "v1", []string{ "certificates.cert-manager.io", "certificaterequests.cert-manager.io", "issuers.cert-manager.io", @@ -120,4 +120,5 @@ func (o *Options) Run(ctx context.Context, args []string) error { "orders.acme.cert-manager.io", "challenges.acme.cert-manager.io", }) + return err } diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go index 12d33628a..1d3f77357 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -24,7 +24,7 @@ type Migrator struct { // If true, skip checking the 'status.storedVersion' before running the migration. // By default, migration will only be run if the CRD contains storedVersions other // than the desired target version. - Force bool + SkipStoredVersionCheck bool // Writers to write informational & error messages to Out, ErrOut io.Writer @@ -32,7 +32,7 @@ type Migrator struct { // NewMigrator creates a new migrator with the given API client. // If either of out or errOut are nil, log messages will be discarded. -func NewMigrator(client client.Client, force bool, out, errOut io.Writer) *Migrator { +func NewMigrator(client client.Client, skipStoredVersionCheck bool, out, errOut io.Writer) *Migrator { if out == nil { out = io.Discard } @@ -41,10 +41,10 @@ func NewMigrator(client client.Client, force bool, out, errOut io.Writer) *Migra } return &Migrator{ - Client: client, - Force: force, - Out: out, - ErrOut: errOut, + Client: client, + SkipStoredVersionCheck: skipStoredVersionCheck, + Out: out, + ErrOut: errOut, } } @@ -65,9 +65,9 @@ func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string fmt.Fprintf(m.Out, "All CustomResourceDefinitions have %q configured as the storage version.\n", targetVersion) crdsRequiringMigration := allCRDs - if !m.Force { - fmt.Fprintln(m.Out, "Looking for CRDs that contain resources that require migrating to 'v1'...") - crdsRequiringMigration, err = m.discoverCRDsRequiringMigration(ctx, "v1", names) + if !m.SkipStoredVersionCheck { + fmt.Fprintf(m.Out, "Looking for CRDs that contain resources that require migrating to %q...\n", targetVersion) + crdsRequiringMigration, err = m.discoverCRDsRequiringMigration(ctx, targetVersion, names) if err != nil { fmt.Fprintf(m.ErrOut, "Failed to determine resource types that require migration: %v\n", err) return err @@ -77,7 +77,7 @@ func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string return nil } } else { - fmt.Fprintln(m.Out, "Forcing migration of all CRD resources as --force=true") + fmt.Fprintln(m.Out, "Forcing migration of all CRD resources as --skip-stored-version-check=true") } fmt.Fprintf(m.Out, "Found %d resource types that require migration:\n", len(crdsRequiringMigration)) From c4d97e1f53129ce8d549ddf9bb7781056fdb7bde Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 15:07:10 +0000 Subject: [PATCH 10/17] Indicate whether a migration was actually performed in Run Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/migrator.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go index 1d3f77357..2fe568f55 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -52,15 +52,16 @@ func NewMigrator(client client.Client, skipStoredVersionCheck bool, out, errOut // It will attempt to migrate all resources defined as part of these CRDs to the // given 'targetVersion', and after completion will update the `status.storedVersions` // field on the corresponding CRD version to only contain the given targetVersion. -func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string) error { +// Returns 'true' if a migration was actually performed, and false if migration was not required. +func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string) (bool, error) { fmt.Fprintf(m.Out, "Checking all CustomResourceDefinitions have storage version set to '%s'\n", targetVersion) - allV1, allCRDs, err := m.ensureCRDStorageVersionEquals(ctx, targetVersion, names) + allTargetVersion, allCRDs, err := m.ensureCRDStorageVersionEquals(ctx, targetVersion, names) if err != nil { - return err + return false, err } - if !allV1 { - fmt.Fprintln(m.ErrOut, "It looks like you are running a pre-1.0 version of cert-manager. Please upgrade cert-manager to v1.6 before upgrading to v1.7.") - return fmt.Errorf("preflight checks failed") + if !allTargetVersion { + fmt.Fprintf(m.ErrOut, "It looks like you are running a version of cert-manager that does not set the storage version of CRDs to %q. Please upgrade cert-manager to v1.6 before upgrading to v1.7.\n", targetVersion) + return false, fmt.Errorf("preflight checks failed") } fmt.Fprintf(m.Out, "All CustomResourceDefinitions have %q configured as the storage version.\n", targetVersion) @@ -70,11 +71,11 @@ func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string crdsRequiringMigration, err = m.discoverCRDsRequiringMigration(ctx, targetVersion, names) if err != nil { fmt.Fprintf(m.ErrOut, "Failed to determine resource types that require migration: %v\n", err) - return err + return false, err } if len(crdsRequiringMigration) == 0 { fmt.Fprintln(m.Out, "Nothing to do. cert-manager CRDs do not have 'status.storedVersions' containing old API versions. You may proceed to upgrade to cert-manager v1.7.") - return nil + return false, nil } } else { fmt.Fprintln(m.Out, "Forcing migration of all CRD resources as --skip-stored-version-check=true") @@ -88,18 +89,18 @@ func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string for _, crd := range crdsRequiringMigration { if err := m.migrateResourcesForCRD(ctx, crd); err != nil { fmt.Fprintf(m.ErrOut, "Failed to migrate resource: %v\n", err) - return err + return false, err } } fmt.Fprintf(m.Out, "Patching CRD resources to set 'status.storedVersions' to %q...\n", targetVersion) if err := m.patchCRDStoredVersions(ctx, crdsRequiringMigration); err != nil { fmt.Fprintf(m.ErrOut, "Failed to patch 'status.storedVersions' field: %v\n", err) - return err + return false, err } fmt.Fprintln(m.Out, "Successfully migrated all cert-manager resource types. It is now safe to proceed with upgrading to cert-manager v1.7.") - return nil + return true, nil } func (m *Migrator) ensureCRDStorageVersionEquals(ctx context.Context, vers string, names []string) (bool, []*apiext.CustomResourceDefinition, error) { From 22b2b3687a2f973c6ef53328c2a4f3cbc605a775 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 15:07:27 +0000 Subject: [PATCH 11/17] Don't assume we are targetting 'v1' Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/migrator.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go index 2fe568f55..e2a3cf248 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -151,7 +151,7 @@ func (m *Migrator) migrateResourcesForCRD(ctx context.Context, crd *apiext.Custo list := &unstructured.UnstructuredList{} list.SetGroupVersionKind(schema.GroupVersionKind{ Group: crd.Spec.Group, - Version: "v1", + Version: storageVersionForCRD(crd), Kind: crd.Spec.Names.ListKind, }) if err := m.Client.List(ctx, list); err != nil { @@ -199,8 +199,8 @@ func (m *Migrator) patchCRDStoredVersions(ctx context.Context, crds []*apiext.Cu return newUnexpectedChangeError(crd) } - // Set the `status.storedVersions` field to 'v1' - freshCRD.Status.StoredVersions = []string{"v1"} + // Set the `status.storedVersions` field to the target storage version + freshCRD.Status.StoredVersions = []string{storageVersionForCRD(crd)} if err := m.Client.Status().Update(ctx, freshCRD); err != nil { return err From 1ae8fedc2dee9cabc1b0db490aedfd4f529f1834 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 15:08:15 +0000 Subject: [PATCH 12/17] Add additional test cases Signed-off-by: James Munnelly --- .../ctl/migrate/ctl_upgrade_migrate_test.go | 200 +++++++++++++++++- 1 file changed, 191 insertions(+), 9 deletions(-) diff --git a/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go index b9f553357..5eb39399e 100644 --- a/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go +++ b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go @@ -39,26 +39,32 @@ import ( "github.com/jetstack/cert-manager/test/integration/framework" ) -var ( - equivalentResources = map[string]client.Object{ - "v1": &v1.TestType{ +// Create a test resource at a given version. +func newResourceAtVersion(t *testing.T, version string) client.Object { + switch version { + case "v1": + return &v1.TestType{ ObjectMeta: metav1.ObjectMeta{ Name: "object", Namespace: "default", }, TestField: "abc", TestFieldImmutable: "def", - }, - "v2": &v2.TestType{ + } + case "v2": + return &v2.TestType{ ObjectMeta: metav1.ObjectMeta{ Name: "object", Namespace: "default", }, TestField: "abc", TestFieldImmutable: "def", - }, + } + default: + t.Fatalf("unknown version %q", version) } -) + return nil +} func newScheme() *runtime.Scheme { scheme := runtime.NewScheme() @@ -113,7 +119,7 @@ func TestCtlUpgradeMigrate(t *testing.T) { } // Create a resource - obj := equivalentResources[storageVersion] + obj := newResourceAtVersion(t, storageVersion) if err := cl.Create(ctx, obj); err != nil { t.Errorf("Failed to create test resource: %v", err) } @@ -129,9 +135,13 @@ func TestCtlUpgradeMigrate(t *testing.T) { // Run the migrator and migrate all objects to the 'nonStorageVersion' (which is now the new storage version) migrator := migrate.NewMigrator(cl, false, os.Stdout, os.Stderr) - if err := migrator.Run(ctx, nonStorageVersion, []string{crdName}); err != nil { + migrated, err := migrator.Run(ctx, nonStorageVersion, []string{crdName}) + if err != nil { t.Errorf("migrator failed to run: %v", err) } + if !migrated { + t.Errorf("migrator didn't actually perform a migration") + } // Check the status.storedVersions field to ensure it only contains one element crd = &apiext.CustomResourceDefinition{} @@ -163,6 +173,178 @@ func TestCtlUpgradeMigrate(t *testing.T) { } } +func TestCtlUpgradeMigrate_FailsIfStorageVersionDoesNotEqualTargetVersion(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + defer cancel() + + // Create the control plane with the TestType conversion handlers registered + scheme := newScheme() + // name of the testtype CRD resource + crdName := "testtypes.testgroup.testing.cert-manager.io" + restCfg, stop := framework.RunControlPlane(t, context.Background(), + framework.WithCRDDirectory("../../../../pkg/webhook/handlers/testdata/apis/testgroup/crds"), + framework.WithWebhookConversionHandler(handlers.NewSchemeBackedConverter(testlogger.NewTestLogger(t), scheme))) + defer stop() + + // Ensure the OpenAPI endpoint has been updated with the TestType CRD + framework.WaitForOpenAPIResourcesToBeLoaded(t, ctx, restCfg, schema.GroupVersionKind{ + Group: "testgroup.testing.cert-manager.io", + Version: "v1", + Kind: "TestType", + }) + + // Create an API client + cl, err := client.New(restCfg, client.Options{Scheme: scheme}) + if err != nil { + t.Fatal(err) + } + + // Fetch a copy of the recently created TestType CRD + crd := &apiext.CustomResourceDefinition{} + if err := cl.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { + t.Fatal(err) + } + + // Identify the current storage version and one non-storage version for this CRD. + storageVersion, nonStorageVersion := versionsForCRD(crd) + if storageVersion == "" || nonStorageVersion == "" { + t.Fatal("this test requires testdata with both a storage and non-storage version set") + } + + // We expect this to fail, as we are attempting to migrate to the 'nonStorageVersion'. + migrator := migrate.NewMigrator(cl, false, os.Stdout, os.Stderr) + migrated, err := migrator.Run(ctx, nonStorageVersion, []string{crdName}) + if err == nil { + t.Errorf("expected an error to be returned but we got none") + } + if err.Error() != "preflight checks failed" { + t.Errorf("unexpected error: %v", err) + } + if migrated { + t.Errorf("migrator ran but it should not have") + } +} + +func TestCtlUpgradeMigrate_SkipsMigrationIfNothingToDo(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + defer cancel() + + // Create the control plane with the TestType conversion handlers registered + scheme := newScheme() + // name of the testtype CRD resource + crdName := "testtypes.testgroup.testing.cert-manager.io" + restCfg, stop := framework.RunControlPlane(t, context.Background(), + framework.WithCRDDirectory("../../../../pkg/webhook/handlers/testdata/apis/testgroup/crds"), + framework.WithWebhookConversionHandler(handlers.NewSchemeBackedConverter(testlogger.NewTestLogger(t), scheme))) + defer stop() + + // Ensure the OpenAPI endpoint has been updated with the TestType CRD + framework.WaitForOpenAPIResourcesToBeLoaded(t, ctx, restCfg, schema.GroupVersionKind{ + Group: "testgroup.testing.cert-manager.io", + Version: "v1", + Kind: "TestType", + }) + + // Create an API client + cl, err := client.New(restCfg, client.Options{Scheme: scheme}) + if err != nil { + t.Fatal(err) + } + + // Fetch a copy of the recently created TestType CRD + crd := &apiext.CustomResourceDefinition{} + if err := cl.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { + t.Fatal(err) + } + + // Identify the current storage version and one non-storage version for this CRD. + storageVersion, nonStorageVersion := versionsForCRD(crd) + if storageVersion == "" || nonStorageVersion == "" { + t.Fatal("this test requires testdata with both a storage and non-storage version set") + } + + // Ensure the original storage version is the only one on the CRD + if len(crd.Status.StoredVersions) != 1 || crd.Status.StoredVersions[0] != storageVersion { + t.Errorf("Expected status.storedVersions to only contain the storage version %q but it was: %v", storageVersion, crd.Status.StoredVersions) + } + + // Create a resource + obj := newResourceAtVersion(t, storageVersion) + if err := cl.Create(ctx, obj); err != nil { + t.Errorf("Failed to create test resource: %v", err) + } + + // We expect this to succeed and for the migration to not be run + migrator := migrate.NewMigrator(cl, false, os.Stdout, os.Stderr) + migrated, err := migrator.Run(ctx, storageVersion, []string{crdName}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if migrated { + t.Errorf("migrator ran but it should not have") + } +} + +func TestCtlUpgradeMigrate_ForcesMigrationIfSkipStoredVersionCheckIsEnabled(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + defer cancel() + + // Create the control plane with the TestType conversion handlers registered + scheme := newScheme() + // name of the testtype CRD resource + crdName := "testtypes.testgroup.testing.cert-manager.io" + restCfg, stop := framework.RunControlPlane(t, context.Background(), + framework.WithCRDDirectory("../../../../pkg/webhook/handlers/testdata/apis/testgroup/crds"), + framework.WithWebhookConversionHandler(handlers.NewSchemeBackedConverter(testlogger.NewTestLogger(t), scheme))) + defer stop() + + // Ensure the OpenAPI endpoint has been updated with the TestType CRD + framework.WaitForOpenAPIResourcesToBeLoaded(t, ctx, restCfg, schema.GroupVersionKind{ + Group: "testgroup.testing.cert-manager.io", + Version: "v1", + Kind: "TestType", + }) + + // Create an API client + cl, err := client.New(restCfg, client.Options{Scheme: scheme}) + if err != nil { + t.Fatal(err) + } + + // Fetch a copy of the recently created TestType CRD + crd := &apiext.CustomResourceDefinition{} + if err := cl.Get(ctx, client.ObjectKey{Name: crdName}, crd); err != nil { + t.Fatal(err) + } + + // Identify the current storage version and one non-storage version for this CRD. + storageVersion, nonStorageVersion := versionsForCRD(crd) + if storageVersion == "" || nonStorageVersion == "" { + t.Fatal("this test requires testdata with both a storage and non-storage version set") + } + + // Ensure the original storage version is the only one on the CRD + if len(crd.Status.StoredVersions) != 1 || crd.Status.StoredVersions[0] != storageVersion { + t.Errorf("Expected status.storedVersions to only contain the storage version %q but it was: %v", storageVersion, crd.Status.StoredVersions) + } + + // Create a resource + obj := newResourceAtVersion(t, storageVersion) + if err := cl.Create(ctx, obj); err != nil { + t.Errorf("Failed to create test resource: %v", err) + } + + // We expect this to force a migration + migrator := migrate.NewMigrator(cl, true, os.Stdout, os.Stderr) + migrated, err := migrator.Run(ctx, storageVersion, []string{crdName}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if !migrated { + t.Errorf("expected migrator to run due to skip flag being set") + } +} + func versionsForCRD(crd *apiext.CustomResourceDefinition) (storage, nonstorage string) { storageVersion := "" nonStorageVersion := "" From 0f199173baf14d2a2bafe5b7ad394a31714d79ff Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 15:08:48 +0000 Subject: [PATCH 13/17] Add missing copyright headers Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrate/migrator.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrate/migrator.go index e2a3cf248..16f2ca2d7 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrate/migrator.go @@ -1,3 +1,19 @@ +/* +Copyright 2022 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package migrate import ( From 35a96362a7297b04b614130c585fe1e97a5745ee Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 15:21:47 +0000 Subject: [PATCH 14/17] Rename command to 'migrate-api-version' Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/BUILD.bazel | 4 ++-- .../upgrade/{migrate => migrateapiversion}/BUILD.bazel | 2 +- .../upgrade/{migrate => migrateapiversion}/command.go | 6 +++--- .../upgrade/{migrate => migrateapiversion}/migrator.go | 2 +- cmd/ctl/pkg/upgrade/upgrade.go | 4 ++-- test/integration/ctl/migrate/BUILD.bazel | 2 +- .../ctl/migrate/ctl_upgrade_migrate_test.go | 10 +++++----- 7 files changed, 15 insertions(+), 15 deletions(-) rename cmd/ctl/pkg/upgrade/{migrate => migrateapiversion}/BUILD.bazel (98%) rename cmd/ctl/pkg/upgrade/{migrate => migrateapiversion}/command.go (97%) rename cmd/ctl/pkg/upgrade/{migrate => migrateapiversion}/migrator.go (99%) diff --git a/cmd/ctl/pkg/upgrade/BUILD.bazel b/cmd/ctl/pkg/upgrade/BUILD.bazel index 765498195..698fb462c 100644 --- a/cmd/ctl/pkg/upgrade/BUILD.bazel +++ b/cmd/ctl/pkg/upgrade/BUILD.bazel @@ -6,7 +6,7 @@ go_library( importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade", visibility = ["//visibility:public"], deps = [ - "//cmd/ctl/pkg/upgrade/migrate:go_default_library", + "//cmd/ctl/pkg/upgrade/migrateapiversion:go_default_library", "@com_github_spf13_cobra//:go_default_library", "@io_k8s_cli_runtime//pkg/genericclioptions:go_default_library", ], @@ -23,7 +23,7 @@ filegroup( name = "all-srcs", srcs = [ ":package-srcs", - "//cmd/ctl/pkg/upgrade/migrate:all-srcs", + "//cmd/ctl/pkg/upgrade/migrateapiversion:all-srcs", ], tags = ["automanaged"], visibility = ["//visibility:public"], diff --git a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel b/cmd/ctl/pkg/upgrade/migrateapiversion/BUILD.bazel similarity index 98% rename from cmd/ctl/pkg/upgrade/migrate/BUILD.bazel rename to cmd/ctl/pkg/upgrade/migrateapiversion/BUILD.bazel index b0d5f869b..83e382363 100644 --- a/cmd/ctl/pkg/upgrade/migrate/BUILD.bazel +++ b/cmd/ctl/pkg/upgrade/migrateapiversion/BUILD.bazel @@ -6,7 +6,7 @@ go_library( "command.go", "migrator.go", ], - importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrate", + importpath = "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrateapiversion", visibility = ["//visibility:public"], deps = [ "//cmd/ctl/pkg/build:go_default_library", diff --git a/cmd/ctl/pkg/upgrade/migrate/command.go b/cmd/ctl/pkg/upgrade/migrateapiversion/command.go similarity index 97% rename from cmd/ctl/pkg/upgrade/migrate/command.go rename to cmd/ctl/pkg/upgrade/migrateapiversion/command.go index 8d9b1baa3..0de16d497 100644 --- a/cmd/ctl/pkg/upgrade/migrate/command.go +++ b/cmd/ctl/pkg/upgrade/migrateapiversion/command.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package migrate +package migrateapiversion import ( "context" @@ -44,7 +44,7 @@ This command must be run with a cluster running cert-manager v1.0 or greater.`)) example = templates.Examples(i18n.T(build.WithTemplate(` # Check the cert-manager installation is ready to be upgraded to v1.7 -{{.BuildName}} upgrade migrate +{{.BuildName}} upgrade migrate-api-version `))) ) @@ -69,7 +69,7 @@ func NewOptions(ioStreams genericclioptions.IOStreams) *Options { func NewCmdMigrate(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.Command { o := NewOptions(ioStreams) cmd := &cobra.Command{ - Use: "migrate", + Use: "migrate-api-version", Short: "Migrate all existing persisted cert-manager resources to the v1 API version", Long: long, Example: example, diff --git a/cmd/ctl/pkg/upgrade/migrate/migrator.go b/cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go similarity index 99% rename from cmd/ctl/pkg/upgrade/migrate/migrator.go rename to cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go index 16f2ca2d7..64211bd44 100644 --- a/cmd/ctl/pkg/upgrade/migrate/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package migrate +package migrateapiversion import ( "context" diff --git a/cmd/ctl/pkg/upgrade/upgrade.go b/cmd/ctl/pkg/upgrade/upgrade.go index 88fb98e31..2688da38d 100644 --- a/cmd/ctl/pkg/upgrade/upgrade.go +++ b/cmd/ctl/pkg/upgrade/upgrade.go @@ -22,7 +22,7 @@ import ( "github.com/spf13/cobra" "k8s.io/cli-runtime/pkg/genericclioptions" - "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrate" + "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrateapiversion" ) func NewCmdUpgrade(ctx context.Context, ioStreams genericclioptions.IOStreams) *cobra.Command { @@ -32,7 +32,7 @@ func NewCmdUpgrade(ctx context.Context, ioStreams genericclioptions.IOStreams) * Long: `Note: this command does NOT actually upgrade cert-manager installations`, } - cmds.AddCommand(migrate.NewCmdMigrate(ctx, ioStreams)) + cmds.AddCommand(migrateapiversion.NewCmdMigrate(ctx, ioStreams)) return cmds } diff --git a/test/integration/ctl/migrate/BUILD.bazel b/test/integration/ctl/migrate/BUILD.bazel index 4477be44a..e26348168 100644 --- a/test/integration/ctl/migrate/BUILD.bazel +++ b/test/integration/ctl/migrate/BUILD.bazel @@ -7,7 +7,7 @@ go_test( "//pkg/webhook/handlers/testdata/apis/testgroup/crds:all-srcs", ], deps = [ - "//cmd/ctl/pkg/upgrade/migrate:go_default_library", + "//cmd/ctl/pkg/upgrade/migrateapiversion:go_default_library", "//pkg/webhook/handlers:go_default_library", "//pkg/webhook/handlers/testdata/apis/testgroup/install:go_default_library", "//pkg/webhook/handlers/testdata/apis/testgroup/v1:go_default_library", diff --git a/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go index 5eb39399e..2144adf00 100644 --- a/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go +++ b/test/integration/ctl/migrate/ctl_upgrade_migrate_test.go @@ -31,7 +31,7 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrate" + "github.com/jetstack/cert-manager/cmd/ctl/pkg/upgrade/migrateapiversion" "github.com/jetstack/cert-manager/pkg/webhook/handlers" "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/install" "github.com/jetstack/cert-manager/pkg/webhook/handlers/testdata/apis/testgroup/v1" @@ -134,7 +134,7 @@ func TestCtlUpgradeMigrate(t *testing.T) { } // Run the migrator and migrate all objects to the 'nonStorageVersion' (which is now the new storage version) - migrator := migrate.NewMigrator(cl, false, os.Stdout, os.Stderr) + migrator := migrateapiversion.NewMigrator(cl, false, os.Stdout, os.Stderr) migrated, err := migrator.Run(ctx, nonStorageVersion, []string{crdName}) if err != nil { t.Errorf("migrator failed to run: %v", err) @@ -212,7 +212,7 @@ func TestCtlUpgradeMigrate_FailsIfStorageVersionDoesNotEqualTargetVersion(t *tes } // We expect this to fail, as we are attempting to migrate to the 'nonStorageVersion'. - migrator := migrate.NewMigrator(cl, false, os.Stdout, os.Stderr) + migrator := migrateapiversion.NewMigrator(cl, false, os.Stdout, os.Stderr) migrated, err := migrator.Run(ctx, nonStorageVersion, []string{crdName}) if err == nil { t.Errorf("expected an error to be returned but we got none") @@ -275,7 +275,7 @@ func TestCtlUpgradeMigrate_SkipsMigrationIfNothingToDo(t *testing.T) { } // We expect this to succeed and for the migration to not be run - migrator := migrate.NewMigrator(cl, false, os.Stdout, os.Stderr) + migrator := migrateapiversion.NewMigrator(cl, false, os.Stdout, os.Stderr) migrated, err := migrator.Run(ctx, storageVersion, []string{crdName}) if err != nil { t.Errorf("unexpected error: %v", err) @@ -335,7 +335,7 @@ func TestCtlUpgradeMigrate_ForcesMigrationIfSkipStoredVersionCheckIsEnabled(t *t } // We expect this to force a migration - migrator := migrate.NewMigrator(cl, true, os.Stdout, os.Stderr) + migrator := migrateapiversion.NewMigrator(cl, true, os.Stdout, os.Stderr) migrated, err := migrator.Run(ctx, storageVersion, []string{crdName}) if err != nil { t.Errorf("unexpected error: %v", err) From 4b257c1d72205ad56a35f123b2ef3f73325ee95d Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 16:39:07 +0000 Subject: [PATCH 15/17] Only mention upgrading to 1.0-1.6 once Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go b/cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go index 64211bd44..50869efb8 100644 --- a/cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go +++ b/cmd/ctl/pkg/upgrade/migrateapiversion/migrator.go @@ -76,7 +76,7 @@ func (m *Migrator) Run(ctx context.Context, targetVersion string, names []string return false, err } if !allTargetVersion { - fmt.Fprintf(m.ErrOut, "It looks like you are running a version of cert-manager that does not set the storage version of CRDs to %q. Please upgrade cert-manager to v1.6 before upgrading to v1.7.\n", targetVersion) + fmt.Fprintf(m.ErrOut, "It looks like you are running a version of cert-manager that does not set the storage version of CRDs to %q. You MUST upgrade to cert-manager v1.0-v1.6 before migrating resources for v1.7.\n", targetVersion) return false, fmt.Errorf("preflight checks failed") } fmt.Fprintf(m.Out, "All CustomResourceDefinitions have %q configured as the storage version.\n", targetVersion) @@ -131,7 +131,7 @@ func (m *Migrator) ensureCRDStorageVersionEquals(ctx context.Context, vers strin storageVersion := storageVersionForCRD(crd) if storageVersion != vers { - fmt.Fprintf(m.Out, "CustomResourceDefinition object %q has storage version set to %q. You MUST upgrade to cert-manager v1.0-v1.6 before migrating resources for v1.7.\n", crdName, storageVersion) + fmt.Fprintf(m.Out, "CustomResourceDefinition object %q has storage version set to %q.\n", crdName, storageVersion) return false, nil, nil } From b3f14ef51d5a8d742cb6fb58a922c30419ca77be Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 16:42:39 +0000 Subject: [PATCH 16/17] Expand example usage text Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrateapiversion/command.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/cmd/ctl/pkg/upgrade/migrateapiversion/command.go b/cmd/ctl/pkg/upgrade/migrateapiversion/command.go index 0de16d497..282d8e36f 100644 --- a/cmd/ctl/pkg/upgrade/migrateapiversion/command.go +++ b/cmd/ctl/pkg/upgrade/migrateapiversion/command.go @@ -43,8 +43,15 @@ This must be run prior to upgrading to ensure your cluster is ready to upgrade t This command must be run with a cluster running cert-manager v1.0 or greater.`)) example = templates.Examples(i18n.T(build.WithTemplate(` -# Check the cert-manager installation is ready to be upgraded to v1.7 +# Check the cert-manager installation is ready to be upgraded to v1.7 and perform necessary migrations +# to ensure that the kube-apiserver has stored only v1 API versions. {{.BuildName}} upgrade migrate-api-version + +# Force migrations to be run, even if the 'status.storedVersion' field on the CRDs does not contain +# old, deprecated API versions. +# This should only be used if you have manually edited/patched the CRDs already. +# It will force a read and a write of ALL cert-manager resources unconditionally. +{{.BuildName}} upgrade migrate-api-version --skip-stored-version-check `))) ) From c841d01d68863e240b87f0ed107c3428385be0f3 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 6 Jan 2022 16:47:43 +0000 Subject: [PATCH 17/17] Allow specify --qps and --burst Signed-off-by: James Munnelly --- cmd/ctl/pkg/upgrade/migrateapiversion/command.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/cmd/ctl/pkg/upgrade/migrateapiversion/command.go b/cmd/ctl/pkg/upgrade/migrateapiversion/command.go index 282d8e36f..9d4a14691 100644 --- a/cmd/ctl/pkg/upgrade/migrateapiversion/command.go +++ b/cmd/ctl/pkg/upgrade/migrateapiversion/command.go @@ -62,6 +62,8 @@ type Options struct { client client.Client skipStoredVersionCheck bool + qps float32 + burst int } // NewOptions returns initialized Options @@ -90,7 +92,8 @@ func NewCmdMigrate(ctx context.Context, ioStreams genericclioptions.IOStreams) * cmd.Flags().BoolVar(&o.skipStoredVersionCheck, "skip-stored-version-check", o.skipStoredVersionCheck, ""+ "If true, all resources will be read and written regardless of the 'status.storedVersions' on the CRD resource. "+ "Use this mode if you have previously manually modified the 'status.storedVersions' field on CRD resources.") - + cmd.Flags().Float32Var(&o.qps, "qps", 5, "Indicates the maximum QPS to the apiserver from the client.") + cmd.Flags().IntVar(&o.burst, "burst", 10, "Maximum burst value for queries set to the apiserver from the client.") o.Factory = factory.New(ctx, cmd) return cmd @@ -109,6 +112,12 @@ func (o *Options) Complete() error { cminstall.Install(scheme) acmeinstall.Install(scheme) + if o.qps != 0 { + o.RESTConfig.QPS = o.qps + } + if o.burst != 0 { + o.RESTConfig.Burst = o.burst + } o.client, err = client.New(o.RESTConfig, client.Options{Scheme: scheme}) if err != nil { return err