Fix: fix uninstallation continues when answer is no (#4710)

Signed-off-by: Charlie Chiang <charlie_c_0129@outlook.com>
(cherry picked from commit 81115ef6ff)

Co-authored-by: Charlie Chiang <charlie_c_0129@outlook.com>

cmd/core/main.go

@@ -142,6 +142,7 @@ func main() {
 	flag.DurationVar(&clusterMetricsInterval, "cluster-metrics-interval", 15*time.Second, "The interval that ClusterMetricsMgr will collect metrics from clusters, default value is 15 seconds.")
 	flag.BoolVar(&controllerArgs.EnableCompatibility, "enable-asi-compatibility", false, "enable compatibility for asi")
 	flag.BoolVar(&controllerArgs.IgnoreAppWithoutControllerRequirement, "ignore-app-without-controller-version", false, "If true, application controller will not process the app without 'app.oam.dev/controller-version-require' annotation")
+	flag.BoolVar(&controllerArgs.IgnoreDefinitionWithoutControllerRequirement, "ignore-definition-without-controller-version", false, "If true, trait/component/workflowstep definition controller will not process the definition without 'definition.oam.dev/controller-version-require' annotation")
 	standardcontroller.AddOptimizeFlags()
 	standardcontroller.AddAdmissionFlags()
 	flag.IntVar(&resourcekeeper.MaxDispatchConcurrent, "max-dispatch-concurrent", 10, "Set the max dispatch concurrent number, default is 10")

pkg/controller/core.oam.dev/oamruntime_controller.go

@@ -86,4 +86,7 @@ type Args struct {
 
 	// IgnoreAppWithoutControllerRequirement indicates that application controller will not process the app without 'app.oam.dev/controller-version-require' annotation.
 	IgnoreAppWithoutControllerRequirement bool
+
+	// IgnoreDefinitionWithoutControllerRequirement indicates that trait/component/workflowstep definition controller will not process the definition without 'definition.oam.dev/controller-version-require' annotation.
+	IgnoreDefinitionWithoutControllerRequirement bool
 }

pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_controller.go

@@ -43,17 +43,24 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/version"
 )
 
 // Reconciler reconciles a ComponentDefinition object
 type Reconciler struct {
 	client.Client
-	dm                   discoverymapper.DiscoveryMapper
-	pd                   *packages.PackageDiscover
-	Scheme               *runtime.Scheme
-	record               event.Recorder
+	dm     discoverymapper.DiscoveryMapper
+	pd     *packages.PackageDiscover
+	Scheme *runtime.Scheme
+	record event.Recorder
+	options
+}
+
+type options struct {
 	defRevLimit          int
 	concurrentReconciles int
+	ignoreDefNoCtrlReq   bool
+	controllerVersion    string
 }
 
 // Reconcile is the main logic for ComponentDefinition controller
@@ -68,6 +75,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	if !r.matchControllerRequirement(&componentDefinition) {
+		klog.InfoS("skip componentDefinition: not match the controller requirement of componentDefinition", "componentDefinition", klog.KObj(&componentDefinition))
+		return ctrl.Result{}, nil
+	}
+
 	// refresh package discover when componentDefinition is registered
 	if componentDefinition.Spec.Workload.Type != types.AutoDetectWorkloadDefinition {
 		err := utils.RefreshPackageDiscover(ctx, r.Client, r.dm, r.pd, &componentDefinition)
@@ -187,12 +199,32 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 // Setup adds a controller that reconciles ComponentDefinition.
 func Setup(mgr ctrl.Manager, args oamctrl.Args) error {
 	r := Reconciler{
-		Client:               mgr.GetClient(),
-		Scheme:               mgr.GetScheme(),
-		dm:                   args.DiscoveryMapper,
-		pd:                   args.PackageDiscover,
-		defRevLimit:          args.DefRevisionLimit,
-		concurrentReconciles: args.ConcurrentReconciles,
+		Client:  mgr.GetClient(),
+		Scheme:  mgr.GetScheme(),
+		dm:      args.DiscoveryMapper,
+		pd:      args.PackageDiscover,
+		options: parseOptions(args),
 	}
 	return r.SetupWithManager(mgr)
 }
+
+func parseOptions(args oamctrl.Args) options {
+	return options{
+		defRevLimit:          args.DefRevisionLimit,
+		concurrentReconciles: args.ConcurrentReconciles,
+		ignoreDefNoCtrlReq:   args.IgnoreDefinitionWithoutControllerRequirement,
+		controllerVersion:    version.VelaVersion,
+	}
+}
+
+func (r *Reconciler) matchControllerRequirement(componentDefinition *v1beta1.ComponentDefinition) bool {
+	if componentDefinition.Annotations != nil {
+		if requireVersion, ok := componentDefinition.Annotations[oam.AnnotationControllerRequirement]; ok {
+			return requireVersion == r.controllerVersion
+		}
+	}
+	if r.ignoreDefNoCtrlReq {
+		return false
+	}
+	return true
+}

pkg/controller/core.oam.dev/v1alpha2/core/components/componentdefinition/componentdefinition_suit_test.go

@@ -90,11 +90,13 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(err).ToNot(HaveOccurred())
 
 	r = Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          dm,
-		pd:          pd,
-		defRevLimit: defRevisionLimit,
+		Client: mgr.GetClient(),
+		Scheme: mgr.GetScheme(),
+		dm:     dm,
+		pd:     pd,
+		options: options{
+			defRevLimit: defRevisionLimit,
+		},
 	}
 	Expect(r.SetupWithManager(mgr)).ToNot(HaveOccurred())
 	var ctx context.Context

pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_controller.go

@@ -42,17 +42,24 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/version"
 )
 
 // Reconciler reconciles a TraitDefinition object
 type Reconciler struct {
 	client.Client
-	dm                   discoverymapper.DiscoveryMapper
-	pd                   *packages.PackageDiscover
-	Scheme               *runtime.Scheme
-	record               event.Recorder
+	dm     discoverymapper.DiscoveryMapper
+	pd     *packages.PackageDiscover
+	Scheme *runtime.Scheme
+	record event.Recorder
+	options
+}
+
+type options struct {
 	defRevLimit          int
 	concurrentReconciles int
+	ignoreDefNoCtrlReq   bool
+	controllerVersion    string
 }
 
 // Reconcile is the main logic for TraitDefinition controller
@@ -67,6 +74,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	if !r.matchControllerRequirement(&traitdefinition) {
+		klog.InfoS("skip traitDefinition: not match the controller requirement of traitDefinition", "traitDefinition", klog.KObj(&traitdefinition))
+		return ctrl.Result{}, nil
+	}
+
 	// this is a placeholder for finalizer here in the future
 	if traitdefinition.DeletionTimestamp != nil {
 		klog.InfoS("The TraitDefinition is being deleted", "traitDefinition", klog.KRef(req.Namespace, req.Name))
@@ -193,12 +205,32 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 // Setup adds a controller that reconciles TraitDefinition.
 func Setup(mgr ctrl.Manager, args oamctrl.Args) error {
 	r := Reconciler{
-		Client:               mgr.GetClient(),
-		Scheme:               mgr.GetScheme(),
-		dm:                   args.DiscoveryMapper,
-		pd:                   args.PackageDiscover,
-		defRevLimit:          args.DefRevisionLimit,
-		concurrentReconciles: args.ConcurrentReconciles,
+		Client:  mgr.GetClient(),
+		Scheme:  mgr.GetScheme(),
+		dm:      args.DiscoveryMapper,
+		pd:      args.PackageDiscover,
+		options: parseOptions(args),
 	}
 	return r.SetupWithManager(mgr)
 }
+
+func parseOptions(args oamctrl.Args) options {
+	return options{
+		defRevLimit:          args.DefRevisionLimit,
+		concurrentReconciles: args.ConcurrentReconciles,
+		ignoreDefNoCtrlReq:   args.IgnoreDefinitionWithoutControllerRequirement,
+		controllerVersion:    version.VelaVersion,
+	}
+}
+
+func (r *Reconciler) matchControllerRequirement(traitDefinition *v1beta1.TraitDefinition) bool {
+	if traitDefinition.Annotations != nil {
+		if requireVersion, ok := traitDefinition.Annotations[oam.AnnotationControllerRequirement]; ok {
+			return requireVersion == r.controllerVersion
+		}
+	}
+	if r.ignoreDefNoCtrlReq {
+		return false
+	}
+	return true
+}

pkg/controller/core.oam.dev/v1alpha2/core/traits/traitdefinition/traitdefinition_suite_test.go

@@ -90,11 +90,13 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(err).ToNot(HaveOccurred())
 
 	r = Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          dm,
-		pd:          pd,
-		defRevLimit: defRevisionLimit,
+		Client: mgr.GetClient(),
+		Scheme: mgr.GetScheme(),
+		dm:     dm,
+		pd:     pd,
+		options: options{
+			defRevLimit: defRevisionLimit,
+		},
 	}
 	Expect(r.SetupWithManager(mgr)).ToNot(HaveOccurred())
 	var ctx context.Context

pkg/controller/core.oam.dev/v1alpha2/core/workflow/workflowstepdefinition/suite_test.go

@@ -90,11 +90,13 @@ var _ = BeforeSuite(func(done Done) {
 	Expect(err).ToNot(HaveOccurred())
 
 	r = Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          dm,
-		pd:          pd,
-		defRevLimit: defRevisionLimit,
+		Client: mgr.GetClient(),
+		Scheme: mgr.GetScheme(),
+		dm:     dm,
+		pd:     pd,
+		options: options{
+			defRevLimit: defRevisionLimit,
+		},
 	}
 	Expect(r.SetupWithManager(mgr)).ToNot(HaveOccurred())
 	var ctx context.Context

pkg/controller/core.oam.dev/v1alpha2/core/workflow/workflowstepdefinition/workflowstepdefinition_controller.go

@@ -42,17 +42,24 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/version"
 )
 
 // Reconciler reconciles a WorkflowStepDefinition object
 type Reconciler struct {
 	client.Client
-	dm                   discoverymapper.DiscoveryMapper
-	pd                   *packages.PackageDiscover
-	Scheme               *runtime.Scheme
-	record               event.Recorder
+	dm     discoverymapper.DiscoveryMapper
+	pd     *packages.PackageDiscover
+	Scheme *runtime.Scheme
+	record event.Recorder
+	options
+}
+
+type options struct {
 	defRevLimit          int
 	concurrentReconciles int
+	ignoreDefNoCtrlReq   bool
+	controllerVersion    string
 }
 
 // Reconcile is the main logic for WorkflowStepDefinition controller
@@ -68,6 +75,11 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		return ctrl.Result{}, client.IgnoreNotFound(err)
 	}
 
+	if !r.matchControllerRequirement(&wfstepdefinition) {
+		klog.InfoS("skip workflowStepDefinition: not match the controller requirement of workflowStepDefinition", "workflowStepDefinition", klog.KObj(&wfstepdefinition))
+		return ctrl.Result{}, nil
+	}
+
 	// this is a placeholder for finalizer here in the future
 	if wfstepdefinition.DeletionTimestamp != nil {
 		return ctrl.Result{}, nil
@@ -192,11 +204,32 @@ func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
 // Setup adds a controller that reconciles WorkflowStepDefinition.
 func Setup(mgr ctrl.Manager, args oamctrl.Args) error {
 	r := Reconciler{
-		Client:      mgr.GetClient(),
-		Scheme:      mgr.GetScheme(),
-		dm:          args.DiscoveryMapper,
-		pd:          args.PackageDiscover,
-		defRevLimit: args.DefRevisionLimit,
+		Client:  mgr.GetClient(),
+		Scheme:  mgr.GetScheme(),
+		dm:      args.DiscoveryMapper,
+		pd:      args.PackageDiscover,
+		options: parseOptions(args),
 	}
 	return r.SetupWithManager(mgr)
 }
+
+func parseOptions(args oamctrl.Args) options {
+	return options{
+		defRevLimit:          args.DefRevisionLimit,
+		concurrentReconciles: args.ConcurrentReconciles,
+		ignoreDefNoCtrlReq:   args.IgnoreDefinitionWithoutControllerRequirement,
+		controllerVersion:    version.VelaVersion,
+	}
+}
+
+func (r *Reconciler) matchControllerRequirement(wfstepdefinition *v1beta1.WorkflowStepDefinition) bool {
+	if wfstepdefinition.Annotations != nil {
+		if requireVersion, ok := wfstepdefinition.Annotations[oam.AnnotationControllerRequirement]; ok {
+			return requireVersion == r.controllerVersion
+		}
+	}
+	if r.ignoreDefNoCtrlReq {
+		return false
+	}
+	return true
+}

pkg/oam/labels.go

@@ -196,7 +196,7 @@ const (
 	// AnnotationWorkloadName indicates the managed workload's name by trait
 	AnnotationWorkloadName = "trait.oam.dev/workload-name"
 
-	// AnnotationControllerRequirement indicates the controller version that can process the application.
+	// AnnotationControllerRequirement indicates the controller version that can process the application/definition.
 	AnnotationControllerRequirement = "app.oam.dev/controller-version-require"
 
 	// AnnotationServiceAccountName indicates the name of the ServiceAccount to use to apply Components and run Workflow.

references/cli/uninstall.go

@@ -17,6 +17,7 @@ limitations under the License.
 package cli
 
 import (
+	"bufio"
 	"context"
 	"fmt"
 	"time"
@@ -48,11 +49,15 @@ type UnInstallArgs struct {
 	Namespace  string
 	Detail     bool
 	force      bool
+	cancel     bool
 }
 
 // NewUnInstallCommand creates `uninstall` command to uninstall vela core
 func NewUnInstallCommand(c common.Args, order string, ioStreams util.IOStreams) *cobra.Command {
-	unInstallArgs := &UnInstallArgs{Args: c, userInput: NewUserInput(), helmHelper: helm.NewHelper()}
+	unInstallArgs := &UnInstallArgs{Args: c, userInput: &UserInput{
+		Writer: ioStreams.Out,
+		Reader: bufio.NewReader(ioStreams.In),
+	}, helmHelper: helm.NewHelper()}
 	cmd := &cobra.Command{
 		Use:     "uninstall",
 		Short:   "Uninstalls KubeVela from a Kubernetes cluster",
@@ -60,8 +65,8 @@ func NewUnInstallCommand(c common.Args, order string, ioStreams util.IOStreams)
 		Long:    "Uninstalls KubeVela from a Kubernetes cluster.",
 		Args:    cobra.ExactArgs(0),
 		PreRunE: func(cmd *cobra.Command, args []string) error {
-			userConfirmation := unInstallArgs.userInput.AskBool("Would you like to uninstall KubeVela from this cluster?", &UserInputOptions{AssumeYes: assumeYes})
-			if !userConfirmation {
+			unInstallArgs.cancel = unInstallArgs.userInput.AskBool("Would you like to uninstall KubeVela from this cluster?", &UserInputOptions{AssumeYes: assumeYes})
+			if !unInstallArgs.cancel {
 				return nil
 			}
 			kubeClient, err := c.GetClient()
@@ -98,6 +103,9 @@ func NewUnInstallCommand(c common.Args, order string, ioStreams util.IOStreams)
 			return nil
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
+			if !unInstallArgs.cancel {
+				return nil
+			}
 			ioStreams.Info("Starting to uninstall KubeVela")
 			restConfig, err := c.GetConfig()
 			if err != nil {

references/cli/uninstall_test.go

@@ -19,14 +19,20 @@ package cli
 import (
 	"context"
 	"fmt"
+	"os"
+	"strings"
+	"testing"
 	"time"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"github.com/stretchr/testify/assert"
 	"sigs.k8s.io/yaml"
 
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
+	"github.com/oam-dev/kubevela/pkg/utils/common"
+	pkgutils "github.com/oam-dev/kubevela/pkg/utils/util"
 )
 
 var _ = Describe("Test Install Command", func() {
@@ -63,6 +69,17 @@ var _ = Describe("Test Install Command", func() {
 	})
 })
 
+func TestUninstall(t *testing.T) {
+	// Test answering NO when prompted. Should just exit.
+	cmd := NewUnInstallCommand(common.Args{}, "", pkgutils.IOStreams{
+		Out: os.Stdout,
+		In:  strings.NewReader("n\n"),
+	})
+	cmd.SetArgs([]string{})
+	err := cmd.Execute()
+	assert.Nil(t, err, "should just exit if answer is no")
+}
+
 var fluxcdYaml = `
 apiVersion: core.oam.dev/v1beta1
 kind: Application