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.

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

karmadactl cordon&uncordon uses factory to access cluster #2611

Merged
merged 1 commit into from
Oct 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 18 additions & 22 deletions pkg/karmadactl/cordon.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
karmadaclientset "github.com/karmada-io/karmada/pkg/generated/clientset/versioned"
"github.com/karmada-io/karmada/pkg/karmadactl/options"
"github.com/karmada-io/karmada/pkg/karmadactl/util"
)

var (
Expand All @@ -42,7 +42,7 @@ const (
)

// NewCmdCordon defines the `cordon` command that mark cluster as unschedulable.
func NewCmdCordon(karmadaConfig KarmadaConfig, parentCommand string) *cobra.Command {
func NewCmdCordon(f util.Factory, parentCommand string) *cobra.Command {
opts := CommandCordonOption{}
cmd := &cobra.Command{
Use: "cordon CLUSTER",
Expand All @@ -55,23 +55,24 @@ func NewCmdCordon(karmadaConfig KarmadaConfig, parentCommand string) *cobra.Comm
if err := opts.Complete(args); err != nil {
return err
}
if err := RunCordonOrUncordon(DesiredCordon, karmadaConfig, opts); err != nil {
if err := RunCordonOrUncordon(DesiredCordon, f, opts); err != nil {
return err
}
return nil
},
}

flags := cmd.Flags()
opts.GlobalCommandOptions.AddFlags(flags)

flags.BoolVar(&opts.DryRun, "dry-run", false, "Run the command in dry-run mode, without making any server requests.")
flags.StringVar(defaultConfigFlags.KubeConfig, "kubeconfig", *defaultConfigFlags.KubeConfig, "Path to the kubeconfig file to use for CLI requests.")
Copy link
Member

Choose a reason for hiding this comment

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

These code comes multiple times, we may extract them as one function.

Copy link
Member

Choose a reason for hiding this comment

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

cc @carlory can we extract these duplicated codes ?

Copy link
Member

Choose a reason for hiding this comment

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

I agree with you.

@helen-frank

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I'll solve this problem after all my pr's are incorporated.

flags.StringVar(defaultConfigFlags.Context, "karmada-context", *defaultConfigFlags.Context, "The name of the kubeconfig context to use")
flags.StringVarP(defaultConfigFlags.Namespace, "namespace", "n", *defaultConfigFlags.Namespace, "If present, the namespace scope for this CLI request")

return cmd
}

// NewCmdUncordon defines the `uncordon` command that mark cluster as schedulable.
func NewCmdUncordon(karmadaConfig KarmadaConfig, parentCommand string) *cobra.Command {
func NewCmdUncordon(f util.Factory, parentCommand string) *cobra.Command {
opts := CommandCordonOption{}
cmd := &cobra.Command{
Use: "uncordon CLUSTER",
Expand All @@ -84,24 +85,23 @@ func NewCmdUncordon(karmadaConfig KarmadaConfig, parentCommand string) *cobra.Co
if err := opts.Complete(args); err != nil {
return err
}
if err := RunCordonOrUncordon(DesiredUnCordon, karmadaConfig, opts); err != nil {
if err := RunCordonOrUncordon(DesiredUnCordon, f, opts); err != nil {
return err
}
return nil
},
}

flags := cmd.Flags()
opts.AddFlags(flags)
flags.StringVar(defaultConfigFlags.KubeConfig, "kubeconfig", *defaultConfigFlags.KubeConfig, "Path to the kubeconfig file to use for CLI requests.")
flags.StringVar(defaultConfigFlags.Context, "karmada-context", *defaultConfigFlags.Context, "The name of the kubeconfig context to use")
flags.StringVarP(defaultConfigFlags.Namespace, "namespace", "n", *defaultConfigFlags.Namespace, "If present, the namespace scope for this CLI request")

return cmd
}

// CommandCordonOption holds all command options for cordon and uncordon
type CommandCordonOption struct {
// global flags
options.GlobalCommandOptions

// ClusterName is the cluster's name that we are going to join with.
ClusterName string

Expand Down Expand Up @@ -167,8 +167,8 @@ func (c *CordonHelper) hasUnschedulerTaint() bool {
// PatchOrReplace uses given karmada clientset to update the cluster unschedulable scheduler, either by patching or
// updating the given cluster object; it may return error if the object cannot be encoded as
// JSON, or if either patch or update calls fail; it will also return error whenever creating a patch has failed
func (c *CordonHelper) PatchOrReplace(controlPlaneClient *karmadaclientset.Clientset) error {
client := controlPlaneClient.ClusterV1alpha1().Clusters()
func (c *CordonHelper) PatchOrReplace(karmadaClient karmadaclientset.Interface) error {
client := karmadaClient.ClusterV1alpha1().Clusters()
oldData, err := json.Marshal(c.cluster)
if err != nil {
return err
Expand Down Expand Up @@ -209,22 +209,18 @@ func (c *CordonHelper) PatchOrReplace(controlPlaneClient *karmadaclientset.Clien

// RunCordonOrUncordon exec marks the cluster unschedulable or schedulable according to desired.
// if true cordon cluster otherwise uncordon cluster.
func RunCordonOrUncordon(desired int, karmadaConfig KarmadaConfig, opts CommandCordonOption) error {
func RunCordonOrUncordon(desired int, f util.Factory, opts CommandCordonOption) error {
cordonOrUncordon := "cordon"
if desired == DesiredUnCordon {
cordonOrUncordon = "un" + cordonOrUncordon
}

// Get control plane kube-apiserver client
controlPlaneRestConfig, err := karmadaConfig.GetRestConfig(opts.KarmadaContext, opts.KubeConfig)
karmadaClient, err := f.KarmadaClientSet()
if err != nil {
return fmt.Errorf("failed to get control plane rest config. context: %s, kube-config: %s, error: %v",
opts.KarmadaContext, opts.KubeConfig, err)
return err
}

controlPlaneKarmadaClient := karmadaclientset.NewForConfigOrDie(controlPlaneRestConfig)

cluster, err := controlPlaneKarmadaClient.ClusterV1alpha1().Clusters().Get(context.TODO(), opts.ClusterName, metav1.GetOptions{})
cluster, err := karmadaClient.ClusterV1alpha1().Clusters().Get(context.TODO(), opts.ClusterName, metav1.GetOptions{})
if err != nil {
return err
}
Expand All @@ -236,7 +232,7 @@ func RunCordonOrUncordon(desired int, karmadaConfig KarmadaConfig, opts CommandC
}

if !opts.DryRun {
err := cordonHelper.PatchOrReplace(controlPlaneKarmadaClient)
err := cordonHelper.PatchOrReplace(karmadaClient)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/karmadactl/karmadactl.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ func NewKarmadaCtlCommand(cmdUse, parentCommand string) *cobra.Command {
{
Message: "Cluster Management Commands:",
Commands: []*cobra.Command{
NewCmdCordon(karmadaConfig, parentCommand),
NewCmdUncordon(karmadaConfig, parentCommand),
NewCmdCordon(f, parentCommand),
NewCmdUncordon(f, parentCommand),
NewCmdTaint(karmadaConfig, parentCommand),
},
},
Expand Down
16 changes: 8 additions & 8 deletions test/e2e/karmadactl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/cli-runtime/pkg/genericclioptions"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/clientcmd"
"k8s.io/klog/v2"
Expand All @@ -23,6 +24,7 @@ import (
policyv1alpha1 "github.com/karmada-io/karmada/pkg/apis/policy/v1alpha1"
"github.com/karmada-io/karmada/pkg/karmadactl"
"github.com/karmada-io/karmada/pkg/karmadactl/options"
cmdutil "github.com/karmada-io/karmada/pkg/karmadactl/util"
"github.com/karmada-io/karmada/pkg/util"
khelper "github.com/karmada-io/karmada/pkg/util/helper"
"github.com/karmada-io/karmada/pkg/util/names"
Expand Down Expand Up @@ -463,6 +465,7 @@ var _ = framework.SerialDescribe("Karmadactl cordon/uncordon testing", ginkgo.La
var kubeConfigPath string
var clusterContext string
var karmadaConfig karmadactl.KarmadaConfig
var f cmdutil.Factory

ginkgo.BeforeEach(func() {
clusterName = "member-e2e-" + rand.String(3)
Expand All @@ -472,6 +475,9 @@ var _ = framework.SerialDescribe("Karmadactl cordon/uncordon testing", ginkgo.La
clusterContext = fmt.Sprintf("kind-%s", clusterName)

karmadaConfig = karmadactl.NewKarmadaConfig(clientcmd.NewDefaultPathOptions())
defaultConfigFlags := genericclioptions.NewConfigFlags(true).WithDeprecatedPasswordFlag().WithDiscoveryBurst(300).WithDiscoveryQPS(50.0)
defaultConfigFlags.Context = &karmadaContext
f = cmdutil.NewFactory(defaultConfigFlags)
})

ginkgo.BeforeEach(func() {
Expand Down Expand Up @@ -527,13 +533,10 @@ var _ = framework.SerialDescribe("Karmadactl cordon/uncordon testing", ginkgo.La
ginkgo.Context("cordon/uncordon cluster taint check", func() {
ginkgo.BeforeEach(func() {
opts := karmadactl.CommandCordonOption{
GlobalCommandOptions: options.GlobalCommandOptions{
KarmadaContext: karmadaContext,
},
DryRun: false,
ClusterName: clusterName,
}
err := karmadactl.RunCordonOrUncordon(karmadactl.DesiredCordon, karmadaConfig, opts)
err := karmadactl.RunCordonOrUncordon(karmadactl.DesiredCordon, f, opts)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())

})
Expand All @@ -554,13 +557,10 @@ var _ = framework.SerialDescribe("Karmadactl cordon/uncordon testing", ginkgo.La

ginkgo.It(fmt.Sprintf("cluster %s should not have unschedulable:NoSchedule taint", clusterName), func() {
opts := karmadactl.CommandCordonOption{
GlobalCommandOptions: options.GlobalCommandOptions{
KarmadaContext: karmadaContext,
},
DryRun: false,
ClusterName: clusterName,
}
err := karmadactl.RunCordonOrUncordon(karmadactl.DesiredUnCordon, karmadaConfig, opts)
err := karmadactl.RunCordonOrUncordon(karmadactl.DesiredUnCordon, f, opts)
gomega.Expect(err).ShouldNot(gomega.HaveOccurred())

ginkgo.By(fmt.Sprintf("cluster %s taint(unschedulable:NoSchedule) will be removed", clusterName), func() {
Expand Down