Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Confirm before overriding installation by another manager #4355

Merged
merged 2 commits into from Nov 1, 2023
Merged

Confirm before overriding installation by another manager #4355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f15ad9
confirm before overriding installation by another manager
somtochiama Oct 24, 2023
9cd4a72
Display better uninstall prompt if flux is managed by a different tool
somtochiama Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 31 additions & 0 deletions cmd/flux/bootstrap.go
View file
Open in desktop
Expand Up @@ -17,11 +17,15 @@ limitations under the License.
package main

import (
"context"
"crypto/elliptic"
"fmt"
"strings"

"github.com/manifoldco/promptui"
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/api/errors"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/fluxcd/flux2/v2/internal/flags"
"github.com/fluxcd/flux2/v2/internal/utils"
Expand Down Expand Up @@ -72,6 +76,8 @@ type bootstrapFlags struct {
gpgPassphrase string
gpgKeyID string

force bool

commitMessageAppendix string
}

Expand Down Expand Up @@ -129,6 +135,7 @@ func init() {

bootstrapCmd.PersistentFlags().StringVar(&bootstrapArgs.commitMessageAppendix, "commit-message-appendix", "", "string to add to the commit messages, e.g. '[ci skip]'")

bootstrapCmd.PersistentFlags().BoolVar(&bootstrapArgs.force, "force", false, "override existing Flux installation if it's managed by a diffrent tool such as Helm")
bootstrapCmd.PersistentFlags().MarkHidden("manifests")

rootCmd.AddCommand(bootstrapCmd)
Expand Down Expand Up @@ -188,3 +195,27 @@ func mapTeamSlice(s []string, defaultPermission string) map[string]string {

return m
}

// confirmBootstrap gets a confirmation for running bootstrap over an existing Flux installation.
// It returns a nil error if Flux is not installed or the user confirms overriding an existing installation
func confirmBootstrap(ctx context.Context, kubeClient client.Client) error {
installed := true
info, err := getFluxClusterInfo(ctx, kubeClient)
if err != nil {
if !errors.IsNotFound(err) {
return fmt.Errorf("cluster info unavailable: %w", err)
}
installed = false
}

if installed {
err = confirmFluxInstallOverride(info)
if err != nil {
if err == promptui.ErrAbort {
return fmt.Errorf("bootstrap cancelled")
}
return err
}
}
return nil
}
7 changes: 7 additions & 0 deletions cmd/flux/bootstrap_bitbucket_server.go
View file
Open in desktop
Expand Up @@ -124,6 +124,13 @@ func bootstrapBServerCmdRun(cmd *cobra.Command, args []string) error {
return err
}

if !bootstrapArgs.force {
err = confirmBootstrap(ctx, kubeClient)
if err != nil {
return err
}
}

// Manifest base
if ver, err := getVersion(bootstrapArgs.version); err != nil {
return err
Expand Down
7 changes: 7 additions & 0 deletions cmd/flux/bootstrap_git.go
View file
Open in desktop
Expand Up @@ -146,6 +146,13 @@ func bootstrapGitCmdRun(cmd *cobra.Command, args []string) error {
return err
}

if !bootstrapArgs.force {
err = confirmBootstrap(ctx, kubeClient)
if err != nil {
return err
stefanprodan marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Manifest base
if ver, err := getVersion(bootstrapArgs.version); err != nil {
return err
Expand Down
7 changes: 7 additions & 0 deletions cmd/flux/bootstrap_github.go
View file
Open in desktop
Expand Up @@ -128,6 +128,13 @@ func bootstrapGitHubCmdRun(cmd *cobra.Command, args []string) error {
return err
}

if !bootstrapArgs.force {
err = confirmBootstrap(ctx, kubeClient)
if err != nil {
return err
}
}

// Manifest base
if ver, err := getVersion(bootstrapArgs.version); err != nil {
return err
Expand Down
7 changes: 7 additions & 0 deletions cmd/flux/bootstrap_gitlab.go
View file
Open in desktop
Expand Up @@ -145,6 +145,13 @@ func bootstrapGitLabCmdRun(cmd *cobra.Command, args []string) error {
return err
}

if !bootstrapArgs.force {
err = confirmBootstrap(ctx, kubeClient)
if err != nil {
return err
}
}

// Manifest base
if ver, err := getVersion(bootstrapArgs.version); err != nil {
return err
Expand Down
39 changes: 31 additions & 8 deletions cmd/flux/cluster_info.go
View file
Open in desktop
Expand Up @@ -20,8 +20,8 @@ import (
"context"
"fmt"

"github.com/manifoldco/promptui"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"

Expand All @@ -47,12 +47,13 @@ type fluxClusterInfo struct {
}

// getFluxClusterInfo returns information on the Flux installation running on the cluster.
// If the information cannot be retrieved, the boolean return value will be false.
// If an error occurred, the returned error will be non-nil.
//
// This function retrieves the GitRepository CRD from the cluster and checks it
// for a set of labels used to determine the Flux version and how Flux was installed.
func getFluxClusterInfo(ctx context.Context, c client.Client) (fluxClusterInfo, bool, error) {
// It returns the NotFound error from the underlying library if it was unable to find
// the GitRepository CRD and this can be used to check if Flux is installed.
func getFluxClusterInfo(ctx context.Context, c client.Client) (fluxClusterInfo, error) {
var info fluxClusterInfo
crdMetadata := &metav1.PartialObjectMetadata{
TypeMeta: metav1.TypeMeta{
Expand All @@ -64,10 +65,7 @@ func getFluxClusterInfo(ctx context.Context, c client.Client) (fluxClusterInfo,
},
}
if err := c.Get(ctx, client.ObjectKeyFromObject(crdMetadata), crdMetadata); err != nil {
if errors.IsNotFound(err) {
return info, false, nil
}
return info, false, err
return info, err
}

info.version = crdMetadata.Labels["app.kubernetes.io/version"]
Expand All @@ -80,8 +78,33 @@ func getFluxClusterInfo(ctx context.Context, c client.Client) (fluxClusterInfo,
info.bootstrapped = true
}

// the `app.kubernetes.io` label is not set by flux but might be set by other
// tools used to install Flux e.g Helm.
if manager, ok := crdMetadata.Labels["app.kubernetes.io/managed-by"]; ok {
info.managedBy = manager
}
return info, true, nil
return info, nil
somtochiama marked this conversation as resolved.
Show resolved Hide resolved
}

// confirmFluxInstallOverride displays a prompt to the user so that they can confirm before overriding
// a Flux installation. It returns nil if the installation should continue,
// promptui.ErrAbort if the user doesn't confirm, or an error encountered.
func confirmFluxInstallOverride(info fluxClusterInfo) error {
// no need to display prompt if installation is managed by Flux
if installManagedByFlux(info.managedBy) {
return nil
}

display := fmt.Sprintf("Flux %s has been installed on this cluster with %s!", info.version, info.managedBy)
fmt.Fprintln(rootCmd.ErrOrStderr(), display)
prompt := promptui.Prompt{
Label: fmt.Sprintf("Are you sure you want to override the %s installation? Y/N", info.managedBy),
IsConfirm: true,
}
_, err := prompt.Run()
return err
}

func installManagedByFlux(manager string) bool {
return manager == "" || manager == "flux"
}
18 changes: 8 additions & 10 deletions cmd/flux/cluster_info_test.go
View file
Open in desktop
Expand Up @@ -24,6 +24,7 @@ import (

. "github.com/onsi/gomega"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

Expand All @@ -44,12 +45,11 @@ func Test_getFluxClusterInfo(t *testing.T) {
name string
labels map[string]string
wantErr bool
wantBool bool
wantInfo fluxClusterInfo
}{
{
name: "no git repository CRD present",
wantBool: false,
name: "no git repository CRD present",
wantErr: true,
},
{
name: "CRD with kustomize-controller labels",
Expand All @@ -58,7 +58,6 @@ func Test_getFluxClusterInfo(t *testing.T) {
fmt.Sprintf("%s/namespace", kustomizev1.GroupVersion.Group): "flux-system",
"app.kubernetes.io/version": "v2.1.0",
},
wantBool: true,
wantInfo: fluxClusterInfo{
version: "v2.1.0",
bootstrapped: true,
Expand All @@ -72,7 +71,6 @@ func Test_getFluxClusterInfo(t *testing.T) {
"app.kubernetes.io/version": "v2.1.0",
"app.kubernetes.io/managed-by": "flux",
},
wantBool: true,
wantInfo: fluxClusterInfo{
version: "v2.1.0",
bootstrapped: true,
Expand All @@ -85,7 +83,6 @@ func Test_getFluxClusterInfo(t *testing.T) {
"app.kubernetes.io/version": "v2.1.0",
"app.kubernetes.io/managed-by": "helm",
},
wantBool: true,
wantInfo: fluxClusterInfo{
version: "v2.1.0",
managedBy: "helm",
Expand All @@ -94,14 +91,13 @@ func Test_getFluxClusterInfo(t *testing.T) {
{
name: "CRD with no labels",
labels: map[string]string{},
wantBool: true,
wantInfo: fluxClusterInfo{},
},
{
name: "CRD with only version label",
labels: map[string]string{
"app.kubernetes.io/version": "v2.1.0",
},
wantBool: true,
wantInfo: fluxClusterInfo{
version: "v2.1.0",
},
Expand All @@ -120,12 +116,14 @@ func Test_getFluxClusterInfo(t *testing.T) {
}

client := builder.Build()
info, present, err := getFluxClusterInfo(context.Background(), client)
info, err := getFluxClusterInfo(context.Background(), client)
if tt.wantErr {
g.Expect(err).To(HaveOccurred())
g.Expect(errors.IsNotFound(err)).To(BeTrue())
} else {
g.Expect(err).To(Not(HaveOccurred()))
}

g.Expect(present).To(Equal(tt.wantBool))
g.Expect(info).To(BeEquivalentTo(tt.wantInfo))
})
}
Expand Down
27 changes: 23 additions & 4 deletions cmd/flux/install.go
View file
Open in desktop
Expand Up @@ -23,7 +23,9 @@ import (
"path/filepath"
"time"

"github.com/manifoldco/promptui"
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/api/errors"

"github.com/fluxcd/flux2/v2/internal/flags"
"github.com/fluxcd/flux2/v2/internal/utils"
Expand Down Expand Up @@ -72,6 +74,7 @@ type installFlags struct {
tokenAuth bool
clusterDomain string
tolerationKeys []string
force bool
}

var installArgs = NewInstallFlags()
Expand All @@ -98,6 +101,7 @@ func init() {
installCmd.Flags().StringVar(&installArgs.clusterDomain, "cluster-domain", rootArgs.defaults.ClusterDomain, "internal cluster domain")
installCmd.Flags().StringSliceVar(&installArgs.tolerationKeys, "toleration-keys", nil,
"list of toleration keys used to schedule the components pods onto nodes with matching taints")
installCmd.Flags().BoolVar(&installArgs.force, "force", false, "override existing Flux installation if it's managed by a diffrent tool such as Helm")
installCmd.Flags().MarkHidden("manifests")

rootCmd.AddCommand(installCmd)
Expand Down Expand Up @@ -188,13 +192,28 @@ func installCmdRun(cmd *cobra.Command, args []string) error {
return err
}

info, installed, err := getFluxClusterInfo(ctx, kubeClient)
installed := true
info, err := getFluxClusterInfo(ctx, kubeClient)
somtochiama marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return fmt.Errorf("cluster info unavailable: %w", err)
if !errors.IsNotFound(err) {
return fmt.Errorf("cluster info unavailable: %w", err)
}
installed = false
}

if info.bootstrapped {
return fmt.Errorf("this cluster has already been bootstrapped with Flux %s! Please use 'flux bootstrap' to upgrade",
info.version)
}

if installed && info.bootstrapped {
return fmt.Errorf("this cluster has already been bootstrapped with Flux %s! Please use 'flux bootstrap' to upgrade", info.version)
if installed && !installArgs.force {
err := confirmFluxInstallOverride(info)
if err != nil {
if err == promptui.ErrAbort {
return fmt.Errorf("installation cancelled")
}
return err
}
}

applyOutput, err := utils.Apply(ctx, kubeconfigArgs, kubeclientOptions, tmpDir, filepath.Join(tmpDir, manifest.Path))
Expand Down
30 changes: 21 additions & 9 deletions cmd/flux/uninstall.go
View file
Open in desktop
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/manifoldco/promptui"
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/api/errors"

"github.com/fluxcd/flux2/v2/internal/utils"
"github.com/fluxcd/flux2/v2/pkg/uninstall"
Expand Down Expand Up @@ -59,24 +60,35 @@ func init() {
}

func uninstallCmdRun(cmd *cobra.Command, args []string) error {
ctx, cancel := context.WithTimeout(context.Background(), rootArgs.timeout)
defer cancel()

kubeClient, err := utils.KubeClient(kubeconfigArgs, kubeclientOptions)
if err != nil {
return err
}

if !uninstallArgs.dryRun && !uninstallArgs.silent {
info, err := getFluxClusterInfo(ctx, kubeClient)
if err != nil {
if !errors.IsNotFound(err) {
return fmt.Errorf("cluster info unavailable: %w", err)
}
}
Comment on lines +76 to +77
Copy link
Contributor

@darkowlzz darkowlzz Oct 31, 2023 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we know that flux is not installed at all if there's an error and it's not found error. We can print some accurate message and quit early. At present, even if it's not installed, it blindly runs the uninstall operations.

? Are you sure you want to delete Flux and its custom resource definitions? [y/N] y
► deleting toolkit.fluxcd.io finalizers in all namespaces
► deleting toolkit.fluxcd.io custom resource definitions
✗ Namespace/flux-system deletion failed: namespaces "flux-system" not found
✔ uninstall finished

Ending with status code 0 is another issue which can be addressed separately.

Copy link
Member Author

@somtochiama somtochiama Oct 31, 2023 •
edited by darkowlzz

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. Here we know that flux is not installed at all if there's an error and it's not found error.

Should we consider a case where the CRDs might not be present (maybe due to a previous incomplete uninstall) but the flux-system namespace is still present? Returning early will not delete the namespace

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should always uninstall, this saved so many users from Helm uninstall and TF delete partial failures where some objects were stuck. I'm also not for extending this PR to uninstall, let's write an issue for improving the uninstall procedure if we must, and discuss there the specs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can keep the uninstall prompt as it doesn't change any existing uninstall behaviour and only gives the user some more information. I will open a new issue for any other changes around the uninstall procedure.


promptLabel := "Are you sure you want to delete Flux and its custom resource definitions"
if !installManagedByFlux(info.managedBy) {
promptLabel = fmt.Sprintf("Flux is managed by %s! Are you sure you want to delete Flux and its CRDs using Flux CLI", info.managedBy)
}
prompt := promptui.Prompt{
Label: "Are you sure you want to delete Flux and its custom resource definitions",
Label: promptLabel,
IsConfirm: true,
}
if _, err := prompt.Run(); err != nil {
return fmt.Errorf("aborting")
}
}

ctx, cancel := context.WithTimeout(context.Background(), rootArgs.timeout)
defer cancel()

kubeClient, err := utils.KubeClient(kubeconfigArgs, kubeclientOptions)
if err != nil {
return err
}

logger.Actionf("deleting components in %s namespace", *kubeconfigArgs.Namespace)
uninstall.Components(ctx, logger, kubeClient, *kubeconfigArgs.Namespace, uninstallArgs.dryRun)

Expand Down