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

Adjustments to SpecOverride #11761

Merged
merged 2 commits into from
Jun 15, 2021
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
18 changes: 14 additions & 4 deletions cmd/kops/create_cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"strings"

"github.com/spf13/cobra"
"github.com/spf13/pflag"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/klog/v2"
Expand Down Expand Up @@ -70,8 +71,8 @@ type CreateClusterOptions struct {
// SSHPublicKeys is a map of the SSH public keys we should configure; required on AWS, not required on GCE
SSHPublicKeys map[string][]byte

// Overrides allows setting values directly in the spec.
Overrides []string
// Sets allows setting values directly in the spec.
Sets []string
// Unsets allows unsetting values directly in the spec.
Unsets []string

Expand Down Expand Up @@ -302,7 +303,7 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
cmd.Flags().StringVarP(&options.Output, "output", "o", options.Output, "Output format. One of json|yaml. Used with the --dry-run flag.")

if featureflag.SpecOverrideFlag.Enabled() {
cmd.Flags().StringSliceVar(&options.Overrides, "override", options.Overrides, "Directly configure values in the spec")
cmd.Flags().StringSliceVar(&options.Sets, "set", options.Sets, "Directly set values in the spec")
cmd.Flags().StringSliceVar(&options.Unsets, "unset", options.Unsets, "Directly unset values in the spec")
}

Expand Down Expand Up @@ -336,6 +337,15 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
cmd.Flags().BoolVar(&options.OpenstackLBOctavia, "os-octavia", options.OpenstackLBOctavia, "If true octavia loadbalancer api will be used")
cmd.Flags().StringVar(&options.OpenstackDNSServers, "os-dns-servers", options.OpenstackDNSServers, "comma separated list of DNS Servers which is used in network")
cmd.Flags().StringVar(&options.OpenstackNetworkID, "os-network", options.OpenstackNetworkID, "The ID of the existing OpenStack network to use")

cmd.Flags().SetNormalizeFunc(func(f *pflag.FlagSet, name string) pflag.NormalizedName {
switch name {
case "override":
name = "set"
}
return pflag.NormalizedName(name)
})

return cmd
}

Expand Down Expand Up @@ -516,7 +526,7 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
if err := commands.UnsetClusterFields(c.Unsets, cluster); err != nil {
return err
}
if err := commands.SetClusterFields(c.Overrides, cluster); err != nil {
if err := commands.SetClusterFields(c.Sets, cluster); err != nil {
return err
}

Expand Down
2 changes: 1 addition & 1 deletion tests/integration/create_cluster/overrides/options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ Zones:
CloudProvider: aws
Networking: cni
KubernetesVersion: v1.21.0
Overrides:
Sets:
- cluster.spec.nodePortAccess=1.2.3.4/32,10.20.30.0/24
6 changes: 3 additions & 3 deletions util/pkg/reflectutils/access.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,12 @@ func setPrimitive(v reflect.Value, newValue string) error {
}

if v.Type().Kind() == reflect.Slice {
// Because this function generally sets values, we overwrite instead of appending.
// Then to support multiple values, we split on commas.
// To support multiple values, we split on commas.
// We have no way to escape a comma currently; but in general we prefer having a slice in the schema,
// rather than having values that need to be parsed, so we may not need it.
tokens := strings.Split(newValue, ",")
valueArray := reflect.MakeSlice(v.Type(), 0, len(tokens))
valueArray := reflect.MakeSlice(v.Type(), 0, v.Len()+len(tokens))
valueArray = reflect.AppendSlice(valueArray, v)
for _, s := range tokens {
valueItem := reflect.New(v.Type().Elem())
if err := setPrimitive(valueItem.Elem(), s); err != nil {
Expand Down
7 changes: 7 additions & 0 deletions util/pkg/reflectutils/access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,13 @@ func TestSet(t *testing.T) {
Path: "spec.containers[0].enumSlice",
Value: "ABC,DEF",
},
{
Name: "append enum slice",
Input: "{ 'spec': { 'containers': [ { 'enumSlice': [ 'ABC', 'DEF' ] } ] } }",
Expected: "{ 'spec': { 'containers': [ { 'enumSlice': [ 'ABC', 'DEF', 'GHI', 'JKL' ] } ] } }",
Path: "spec.containers[0].enumSlice",
Value: "GHI,JKL",
},
// Not sure if we should do this...
// {
// Name: "creating missing array elements",
Expand Down