Skip to content

Commit

Permalink
fix: add enum check of flags of create cmd (#534)
Browse files Browse the repository at this point in the history 
(cherry picked from commit 77d59e5)
  • Loading branch information
@yipeng1030
yipeng1030 committed Jan 8, 2025
1 parent a2dadcf commit 267dc72
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 44 deletions.
139 changes: 95 additions & 44 deletions pkg/cmd/cluster/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,28 +93,8 @@ var _ = Describe("Cluster", func() {
Expect(o.ChartInfo).ShouldNot(BeNil())
o.Format = printer.YAML

Expect(o.CreateOptions.Complete()).To(Succeed())
o.Client = testing.FakeClientSet()
fakeDiscovery1, _ := o.Client.Discovery().(*fakediscovery.FakeDiscovery)
fakeDiscovery1.FakedServerVersion = &version.Info{Major: "1", Minor: "27", GitVersion: "v1.27.0"}
Expect(o.Complete(nil)).To(Succeed())
Expect(o.Validate()).To(Succeed())
Expect(o.Name).ShouldNot(BeEmpty())
Expect(o.Run()).Should(Succeed())
})
It("should apply SharedNode tenancy and Preferred PodAntiAffinity", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.Tenancy = "SharedNode"
o.TopologyKeys = []string{"test-topology1", "test-topology2"}
o.NodeLabels = map[string]string{"environment": "test-env", "region": "test-region"}
o.TolerationsRaw = []string{"testKey1=testValue1:NoSchedule", "testKey2=testValue2:NoExecute"}
o.PodAntiAffinity = "Preferred"

Expect(o).ShouldNot(BeNil())
Expect(o.ChartInfo).ShouldNot(BeNil())
o.Format = printer.YAML

o.Tenancy = "SharedNode"
Expect(o.CreateOptions.Complete()).To(Succeed())
o.Client = testing.FakeClientSet()
fakeDiscovery1, _ := o.Client.Discovery().(*fakediscovery.FakeDiscovery)
Expand All @@ -124,29 +104,6 @@ var _ = Describe("Cluster", func() {
Expect(o.Name).ShouldNot(BeEmpty())
Expect(o.Run()).Should(Succeed())
})

It("should apply DedicatedNode tenancy and Required PodAntiAffinity", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.Tenancy = "DedicatedNode"
o.TopologyKeys = []string{"test-region", "test-zone"}
o.NodeLabels = map[string]string{"cluster": "test-cluster", "env": "test-production"}
o.TolerationsRaw = []string{"testKey3=testValue3:NoSchedule", "testKey4=testValue4:NoExecute"}
o.PodAntiAffinity = "Required"

Expect(o).ShouldNot(BeNil())
Expect(o.ChartInfo).ShouldNot(BeNil())
o.Format = printer.YAML

Expect(o.CreateOptions.Complete()).To(Succeed())
o.Client = testing.FakeClientSet()
fakeDiscovery2, _ := o.Client.Discovery().(*fakediscovery.FakeDiscovery)
fakeDiscovery2.FakedServerVersion = &version.Info{Major: "1", Minor: "27", GitVersion: "v1.27.0"}
Expect(o.Complete(nil)).To(Succeed())
Expect(o.Validate()).To(Succeed())
Expect(o.Name).ShouldNot(BeEmpty())
Expect(o.Run()).Should(Succeed())
})
})

Context("create validate", func() {
Expand All @@ -162,6 +119,8 @@ var _ = Describe("Cluster", func() {
}
o.Name = "mycluster"
o.ChartInfo, _ = cluster.BuildChartInfo(clusterType)
o.PodAntiAffinity = "Preferred"
o.Tenancy = "SharedNode"
})

It("can validate the cluster name must begin with a letter and can only contain lowercase letters, numbers, and '-'.", func() {
Expand Down Expand Up @@ -212,6 +171,98 @@ var _ = Describe("Cluster", func() {
Expect(o.Validate()).Should(HaveOccurred())
})

// Test case: invalid tenancy value
It("should fail when tenancy is invalid", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.Tenancy = "InvalidTenancy" // Set invalid tenancy value

Expect(o.Validate()).ShouldNot(Succeed()) // Validation should fail
})

// Test case: invalid podAntiAffinity value
It("should fail when podAntiAffinity is invalid", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.PodAntiAffinity = "None" // Set invalid podAntiAffinity value

Expect(o.Validate()).ShouldNot(Succeed()) // Validation should fail
})

// Test case: topologyKeys contains empty values
It("should fail when topologyKeys contains empty values", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.TopologyKeys = []string{"", "valid-topology"} // One of the keys is empty

Expect(o.Validate()).ShouldNot(Succeed()) // Validation should fail
})

// Test case: nodeLabels contains empty key or value
It("should fail when nodeLabels contains empty key or value", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.NodeLabels = map[string]string{"environment": "", "": "test-region"} // Key or value is empty

Expect(o.Validate()).ShouldNot(Succeed()) // Validation should fail
})

// Test case: tolerations contains empty key or operator
It("should fail when tolerations contains empty key or operator", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.TolerationsRaw = []string{"testKey=:NoSchedule", "=testValue:NoExecute"} // Key or operator is empty

Expect(o.Validate()).ShouldNot(Succeed()) // Validation should fail
})

// Test case: all inputs are valid
It("should apply SharedNode tenancy and Preferred PodAntiAffinity", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.Tenancy = "SharedNode"
o.TopologyKeys = []string{"test-topology1", "test-topology2"}
o.NodeLabels = map[string]string{"environment": "test-env", "region": "test-region"}
o.TolerationsRaw = []string{"testKey1=testValue1:NoSchedule", "testKey2=testValue2:NoExecute"}
o.PodAntiAffinity = "Preferred"

Expect(o).ShouldNot(BeNil())
Expect(o.ChartInfo).ShouldNot(BeNil())
o.Format = printer.YAML

Expect(o.CreateOptions.Complete()).To(Succeed())
o.Client = testing.FakeClientSet()
fakeDiscovery1, _ := o.Client.Discovery().(*fakediscovery.FakeDiscovery)
fakeDiscovery1.FakedServerVersion = &version.Info{Major: "1", Minor: "27", GitVersion: "v1.27.0"}
Expect(o.Complete(nil)).To(Succeed())
Expect(o.Validate()).To(Succeed())
Expect(o.Name).ShouldNot(BeEmpty())
Expect(o.Run()).Should(Succeed())
})

It("should apply DedicatedNode tenancy and Required PodAntiAffinity", func() {
o, err := NewSubCmdsOptions(createOptions, clusterType)
Expect(err).Should(Succeed())
o.Tenancy = "DedicatedNode"
o.TopologyKeys = []string{"test-region", "test-zone"}
o.NodeLabels = map[string]string{"cluster": "test-cluster", "env": "test-production"}
o.TolerationsRaw = []string{"testKey3=testValue3:NoSchedule", "testKey4=testValue4:NoExecute"}
o.PodAntiAffinity = "Required"

Expect(o).ShouldNot(BeNil())
Expect(o.ChartInfo).ShouldNot(BeNil())
o.Format = printer.YAML

Expect(o.CreateOptions.Complete()).To(Succeed())
o.Client = testing.FakeClientSet()
fakeDiscovery2, _ := o.Client.Discovery().(*fakediscovery.FakeDiscovery)
fakeDiscovery2.FakedServerVersion = &version.Info{Major: "1", Minor: "27", GitVersion: "v1.27.0"}
Expect(o.Complete(nil)).To(Succeed())
Expect(o.Validate()).To(Succeed())
Expect(o.Name).ShouldNot(BeEmpty())
Expect(o.Run()).Should(Succeed())
})

})

Context("delete cluster", func() {
Expand Down
29 changes: 29 additions & 0 deletions pkg/cmd/cluster/create_subcmds.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ func buildCreateSubCmds(createOptions *action.CreateOptions) []*cobra.Command {
// Schedule policy
if cmd.Flag("pod-anti-affinity") == nil {
cmd.Flags().StringVar(&o.PodAntiAffinity, "pod-anti-affinity", "Preferred", "Pod anti-affinity type, one of: (Preferred, Required)")
_ = cmd.Flags().SetAnnotation("pod-anti-affinity", cobra.BashCompCustom, []string{"Preferred", "Required"})
}
if cmd.Flag("topology-keys") == nil {
cmd.Flags().StringArrayVar(&o.TopologyKeys, "topology-keys", nil, "Topology keys for affinity")
Expand All @@ -141,6 +142,7 @@ func buildCreateSubCmds(createOptions *action.CreateOptions) []*cobra.Command {
}
if cmd.Flag("tenancy") == nil {
cmd.Flags().StringVar(&o.Tenancy, "tenancy", "SharedNode", "Tenancy options, one of: (SharedNode, DedicatedNode)")
_ = cmd.Flags().SetAnnotation("tenancy", cobra.BashCompCustom, []string{"SharedNode", "DedicatedNode"})
}
cmds = append(cmds, cmd)
}
Expand Down Expand Up @@ -234,6 +236,33 @@ func (o *CreateSubCmdsOptions) Validate() error {
if len(o.Name) > 16 {
return fmt.Errorf("cluster name should be less than 16 characters")
}
if o.Tenancy != "SharedNode" && o.Tenancy != "DedicatedNode" {
return fmt.Errorf("tenancy must be one of: (SharedNode, DedicatedNode)")
}
if o.PodAntiAffinity != "Preferred" && o.PodAntiAffinity != "Required" {
return fmt.Errorf("podAntiAffinity must be one of: (Preferred, Required)")
}
if o.TopologyKeys != nil {
for _, key := range o.TopologyKeys {
if key == "" {
return fmt.Errorf("topologyKeys cannot be empty")
}
}
}
if o.NodeLabels != nil {
for k, v := range o.NodeLabels {
if k == "" || v == "" {
return fmt.Errorf("nodeLabels cannot be empty")
}
}
}
if o.Tolerations != nil {
for _, t := range o.Tolerations {
if t.Key == "" || t.Operator == "" {
return fmt.Errorf("tolerations cannot be empty")
}
}
}
return cluster.ValidateValues(o.ChartInfo, o.Values)
}

Expand Down
4 changes: 4 additions & 0 deletions pkg/cmd/cluster/create_subcmds_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ var _ = Describe("create cluster by cluster type", func() {
Expect(err).Should(Succeed())
Expect(o).ShouldNot(BeNil())
Expect(o.ChartInfo).ShouldNot(BeNil())
o.PodAntiAffinity = "Preferred"
o.Tenancy = "SharedNode"

By("complete")
var mysqlCmd *cobra.Command
Expand Down Expand Up @@ -139,6 +141,8 @@ var _ = Describe("create cluster by cluster type", func() {
Expect(err).Should(Succeed())
Expect(o).ShouldNot(BeNil())
Expect(o.ChartInfo).ShouldNot(BeNil())
o.PodAntiAffinity = "Preferred"
o.Tenancy = "SharedNode"

By("complete")
var shardCmd *cobra.Command
Expand Down

0 comments on commit 267dc72

Please sign in to comment.