From 59ac3bce6bfef635ca802b574081c0e085bd38a2 Mon Sep 17 00:00:00 2001 From: Sunny Date: Mon, 15 Jun 2020 03:03:21 +0530 Subject: [PATCH] Fix flag overrides --- cmd/ignite/run/create.go | 132 +++++++++++++++++++++++++-------------- e2e/config_test.go | 21 ++++++- 2 files changed, 104 insertions(+), 49 deletions(-) diff --git a/cmd/ignite/run/create.go b/cmd/ignite/run/create.go index 26efff67e..0064e6793 100644 --- a/cmd/ignite/run/create.go +++ b/cmd/ignite/run/create.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "io/ioutil" "path" "strings" @@ -15,6 +16,8 @@ import ( "github.com/weaveworks/ignite/pkg/metadata" "github.com/weaveworks/ignite/pkg/operations" "github.com/weaveworks/ignite/pkg/providers" + + patchutil "github.com/weaveworks/libgitops/pkg/util/patch" ) func NewCreateFlags() *CreateFlags { @@ -43,30 +46,21 @@ type createOptions struct { kernel *api.Kernel } -func (cf *CreateFlags) constructVMFromCLI(args []string) error { - if len(args) == 1 { - ociRef, err := meta.NewOCIImageRef(args[0]) - if err != nil { - return err - } - - cf.VM.Spec.Image.OCI = ociRef - } - +func (cf *CreateFlags) constructVMFromCLI(vm *api.VM, args []string) error { // Parse the --copy-files flag var err error - if cf.VM.Spec.CopyFiles, err = parseFileMappings(cf.CopyFiles); err != nil { + if vm.Spec.CopyFiles, err = parseFileMappings(cf.CopyFiles); err != nil { return err } // Parse the given port mappings - if cf.VM.Spec.Network.Ports, err = meta.ParsePortMappings(cf.PortMappings); err != nil { + if vm.Spec.Network.Ports, err = meta.ParsePortMappings(cf.PortMappings); err != nil { return err } // If the SSH flag was set, copy it over to the API type if cf.SSH.Generate || cf.SSH.PublicKey != "" { - cf.VM.Spec.SSH = &cf.SSH + vm.Spec.SSH = &cf.SSH } // If --require-name is true, VM name must be provided. @@ -78,64 +72,108 @@ func (cf *CreateFlags) constructVMFromCLI(args []string) error { } func (cf *CreateFlags) NewCreateOptions(args []string, fs *flag.FlagSet) (*createOptions, error) { - // If component config file is in use, create a new base VM, use the VM - // spec defined in the config and based on the flags, override the configs. + // Create a new VM and configure it by combining the component config, VM + // config file and flags. + newVM := providers.Client.VMs().New() + + // If component config is in use, use the VM spec defined in the config. if providers.ComponentConfig != nil { - newVM := providers.Client.VMs().New() newVM.Spec = providers.ComponentConfig.Spec.VM + } - if fs.Changed("name") { - newVM.Name = cf.VM.Name - } - - if fs.Changed("cpus") { - newVM.Spec.CPUs = cf.VM.Spec.CPUs + // Image is necessary while serializing the VM spec. Set the passed image + // if provided. + if len(args) == 1 { + ociRef, err := meta.NewOCIImageRef(args[0]) + if err != nil { + return nil, err } + newVM.Spec.Image.OCI = ociRef + } - if fs.Changed("kernel-args") { - newVM.Spec.Kernel.CmdLine = cf.VM.Spec.Kernel.CmdLine + // Decode the config file if given, or construct the VM based off flags and args + if len(cf.ConfigFile) != 0 { + // Marshal into a "clean" object, discard all flag input + fileVM := &api.VM{} + if err := scheme.Serializer.DecodeFileInto(cf.ConfigFile, fileVM); err != nil { + return nil, err } - if fs.Changed("memory") { - newVM.Spec.Memory = cf.VM.Spec.Memory + // Image is necessary while serializing the VM spec. Set use VM image + // provided in the VM config, in case it's not defined in component + // config and not passed as an argument. + if !fileVM.Spec.Image.OCI.IsUnset() { + newVM.Spec.Image.OCI = fileVM.Spec.Image.OCI } - if fs.Changed("size") { - newVM.Spec.DiskSize = cf.VM.Spec.DiskSize - } + p := patchutil.NewPatcher(scheme.Serializer) - if fs.Changed("kernel-image") { - newVM.Spec.Kernel.OCI = cf.VM.Spec.Kernel.OCI + // fileVMBytes, err := scheme.Serializer.EncodeJSON(fileVM) + // if err != nil { + // return nil, err + // } + // Read the yaml config as it is to avoid defaulting the VM + // configurations. + fileVMBytes, err := ioutil.ReadFile(cf.ConfigFile) + if err != nil { + return nil, err } - - if fs.Changed("sandbox-image") { - newVM.Spec.Sandbox.OCI = cf.VM.Spec.Sandbox.OCI + fmt.Println("FILE VM BYTES:", string(fileVMBytes)) + // newVMBytes, err := scheme.Serializer.EncodeJSON(newVM) + newVMBytes, err := scheme.Serializer.EncodeYAML(newVM) + if err != nil { + return nil, err } - - if fs.Changed("volumes") { - newVM.Spec.Storage = cf.VM.Spec.Storage + fmt.Println("NEW VM BYTES:", string(newVMBytes)) + resultVMBytes, err := p.Apply(newVMBytes, fileVMBytes, newVM.GroupVersionKind()) + if err != nil { + return nil, err } + fmt.Println("RESULT VM BYTES:", string(resultVMBytes)) - cf.VM = newVM - } - - // Decode the config file if given, or construct the VM based off flags and args - if len(cf.ConfigFile) != 0 { - // Marshal into a "clean" object, discard all flag input - cf.VM = &api.VM{} - if err := scheme.Serializer.DecodeFileInto(cf.ConfigFile, cf.VM); err != nil { + if err := scheme.Serializer.DecodeInto(resultVMBytes, newVM); err != nil { return nil, err } + // If --require-name is true, VM name must be provided. - if cf.RequireName && len(cf.VM.Name) == 0 { + if cf.RequireName && len(newVM.Name) == 0 { return nil, fmt.Errorf("must set VM name, flag --require-name set") } } else { - if err := cf.constructVMFromCLI(args); err != nil { + if err := cf.constructVMFromCLI(newVM, args); err != nil { return nil, err } } + // Override configs passed through flags. + if fs.Changed("name") { + newVM.Name = cf.VM.Name + } + if fs.Changed("cpus") { + newVM.Spec.CPUs = cf.VM.Spec.CPUs + } + if fs.Changed("kernel-args") { + newVM.Spec.Kernel.CmdLine = cf.VM.Spec.Kernel.CmdLine + } + if fs.Changed("memory") { + newVM.Spec.Memory = cf.VM.Spec.Memory + } + if fs.Changed("size") { + newVM.Spec.DiskSize = cf.VM.Spec.DiskSize + } + if fs.Changed("kernel-image") { + newVM.Spec.Kernel.OCI = cf.VM.Spec.Kernel.OCI + } + if fs.Changed("sandbox-image") { + newVM.Spec.Sandbox.OCI = cf.VM.Spec.Sandbox.OCI + } + if fs.Changed("volumes") { + newVM.Spec.Storage = cf.VM.Spec.Storage + } + + // Assign the new VM to the configFlag. + cf.VM = newVM + // Validate the VM object if err := validation.ValidateVM(cf.VM).ToAggregate(); err != nil { return nil, err diff --git a/e2e/config_test.go b/e2e/config_test.go index b8cd19db3..a239c25eb 100644 --- a/e2e/config_test.go +++ b/e2e/config_test.go @@ -19,6 +19,7 @@ func TestConfigFile(t *testing.T) { cases := []struct { name string config []byte + vmConfig []byte args []string wantVMProperties string err bool @@ -51,10 +52,10 @@ spec: sandbox: oci: weaveworks/ignite:dev kernel: - oci: weaveworks/ignite-kernel:4.19.47 + oci: weaveworks/ignite-kernel:5.4.43 ssh: true `), - wantVMProperties: "'2.0 GB 2 3.0 GB weaveworks/ignite-ubuntu:latest weaveworks/ignite:dev weaveworks/ignite-kernel:4.19.47 {true }'", + wantVMProperties: "'2.0 GB 2 3.0 GB weaveworks/ignite-ubuntu:latest weaveworks/ignite:dev weaveworks/ignite-kernel:5.4.43 {true }'", }, { name: "runtime and network config", @@ -85,9 +86,25 @@ spec: args: []string{"--memory=1GB", "--size=1GB", "--cpus=1", "--ssh"}, wantVMProperties: fmt.Sprintf("'1024.0 MB 1 1024.0 MB weaveworks/ignite-ubuntu:latest weaveworks/ignite:dev weaveworks/ignite-kernel:%s {true }'", constants.DEFAULT_KERNEL_IMAGE_TAG), }, + // { + // name: "vm config", + // config: []byte(`--- + // apiVersion: ignite.weave.works/v1alpha2 + // kind: Configuration + // metadata: + // name: test-config + // spec: + // vm: + // memory: "2GB" + // diskSize: "3GB" + // cpus: 2 + // `), + + // }, } for _, rt := range cases { + rt := rt t.Run(rt.name, func(t *testing.T) { // Create config file. file, err := ioutil.TempFile("", "ignite-config-file-test")