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

virtctl: Apply --namespace to created manifests #10167

Merged
merged 4 commits into from
Sep 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pkg/virtctl/create/BUILD.bazel
Expand Up @@ -12,5 +12,6 @@ go_library(
"//pkg/virtctl/create/vm:go_default_library",
"//pkg/virtctl/templates:go_default_library",
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/k8s.io/client-go/tools/clientcmd:go_default_library",
],
)
9 changes: 5 additions & 4 deletions pkg/virtctl/create/create.go
Expand Up @@ -21,6 +21,7 @@ package create

import (
"github.com/spf13/cobra"
Copy link
Contributor

Choose a reason for hiding this comment

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

newline?

Copy link
Member Author

Choose a reason for hiding this comment

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

Basically the same as the other comment: First stdlib imports, then remote packages, then local packages. I did this on purpose while touching the imports anyways.

"k8s.io/client-go/tools/clientcmd"

"kubevirt.io/kubevirt/pkg/virtctl/create/clone"

Expand All @@ -34,7 +35,7 @@ const (
CREATE = "create"
)

func NewCommand() *cobra.Command {
func NewCommand(clientConfig clientcmd.ClientConfig) *cobra.Command {
cmd := &cobra.Command{
Use: CREATE,
Short: "Create a manifest for the specified Kind.",
Expand All @@ -43,9 +44,9 @@ func NewCommand() *cobra.Command {
},
}

cmd.AddCommand(vm.NewCommand())
cmd.AddCommand(preference.NewCommand())
cmd.AddCommand(instancetype.NewCommand())
cmd.AddCommand(vm.NewCommand(clientConfig))
cmd.AddCommand(preference.NewCommand(clientConfig))
cmd.AddCommand(instancetype.NewCommand(clientConfig))
cmd.AddCommand(clone.NewCommand())
cmd.SetUsageTemplate(templates.UsageTemplate())

Expand Down
1 change: 1 addition & 0 deletions pkg/virtctl/create/instancetype/BUILD.bazel
Expand Up @@ -13,6 +13,7 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library",
"//vendor/k8s.io/client-go/tools/clientcmd:go_default_library",
"//vendor/sigs.k8s.io/yaml:go_default_library",
],
)
Expand Down
37 changes: 31 additions & 6 deletions pkg/virtctl/create/instancetype/instancetype.go
Expand Up @@ -26,6 +26,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/client-go/tools/clientcmd"
v1 "kubevirt.io/api/core/v1"
instancetypev1beta1 "kubevirt.io/api/instancetype/v1beta1"
"sigs.k8s.io/yaml"
Expand All @@ -51,13 +52,16 @@ const (
)

type createInstancetype struct {
namespace string
name string
cpu uint32
memory string
gpus []string
hostDevices []string
ioThreadsPolicy string
namespaced bool

clientConfig clientcmd.ClientConfig
}

type GPU struct {
Expand All @@ -78,8 +82,10 @@ var optFns = map[string]optionFn{
IOThreadsPolicyFlag: withIOThreadsPolicy,
}

func NewCommand() *cobra.Command {
c := createInstancetype{}
func NewCommand(clientConfig clientcmd.ClientConfig) *cobra.Command {
c := createInstancetype{
clientConfig: clientConfig,
}
cmd := &cobra.Command{
Use: Instancetype,
Short: "Create VirtualMachineInstancetype or VirtualMachineClusterInstancetype manifest.",
Expand Down Expand Up @@ -107,16 +113,27 @@ func NewCommand() *cobra.Command {
return cmd
}

func (c *createInstancetype) setDefaults(cmd *cobra.Command) {
func (c *createInstancetype) setDefaults(cmd *cobra.Command) error {
namespace, overridden, err := c.clientConfig.Namespace()
if err != nil {
return err
}
if overridden {
c.namespace = namespace
c.namespaced = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the namespaced flag? We could by default create VirtualMachineClusterInstancetype and based on namespace flag decide if we want the VirtualMachineInstancetype instead. What do you think about that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd keep the namespaced flag so it is still possible to create a VirtualMachineInstancetype that does not have the namespace set.

}

if cmd.Flags().Changed(NameFlag) {
return
return nil
}

if c.namespaced {
c.name = "instancetype-" + rand.String(5)
} else {
c.name = "clusterinstancetype-" + rand.String(5)
}

return nil
}

func withGPUs(c *createInstancetype, instancetypeSpec *instancetypev1beta1.VirtualMachineInstancetypeSpec) error {
Expand Down Expand Up @@ -197,7 +214,7 @@ func (c *createInstancetype) usage() string {
}

func (c *createInstancetype) newInstancetype(_ *cobra.Command) *instancetypev1beta1.VirtualMachineInstancetype {
return &instancetypev1beta1.VirtualMachineInstancetype{
instancetype := &instancetypev1beta1.VirtualMachineInstancetype{
TypeMeta: metav1.TypeMeta{
Kind: "VirtualMachineInstancetype",
APIVersion: instancetypev1beta1.SchemeGroupVersion.String(),
Expand All @@ -214,6 +231,12 @@ func (c *createInstancetype) newInstancetype(_ *cobra.Command) *instancetypev1be
},
},
}

if c.namespace != "" {
instancetype.Namespace = c.namespace
}

return instancetype
}

func (c *createInstancetype) newClusterInstancetype(_ *cobra.Command) *instancetypev1beta1.VirtualMachineClusterInstancetype {
Expand Down Expand Up @@ -264,7 +287,9 @@ func (c *createInstancetype) run(cmd *cobra.Command) error {
var out []byte
var err error

c.setDefaults(cmd)
if err := c.setDefaults(cmd); err != nil {
return err
}

if err := c.validateFlags(); err != nil {
return err
Expand Down
17 changes: 17 additions & 0 deletions pkg/virtctl/create/instancetype/instancetype_test.go
Expand Up @@ -125,6 +125,23 @@ var _ = Describe("create", func() {
Entry("VirtualMachineClusterInstacetype set to shared", "", "shared", false, v1.IOThreadsPolicyShared),
)

It("should create namespaced object and apply namespace when namespace is specified", func() {
const namespace = "my-namespace"
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(create, Instancetype,
setFlag(CPUFlag, "1"),
setFlag(MemoryFlag, "128Mi"),
setFlag("namespace", namespace),
)()
Expect(err).ToNot(HaveOccurred())

decodedObj, err := runtime.Decode(generatedscheme.Codecs.UniversalDeserializer(), bytes)
Expect(err).ToNot(HaveOccurred())

instancetype, ok := decodedObj.(*instancetypev1beta1.VirtualMachineInstancetype)
Expect(ok).To(BeTrue())
Expect(instancetype.Namespace).To(Equal(namespace))
})

DescribeTable("invalid cpu and memory", func(cpu, memory, errMsg string) {
err := clientcmd.NewRepeatableVirtctlCommand(create, Instancetype, namespaced,
setFlag(CPUFlag, cpu),
Expand Down
1 change: 1 addition & 0 deletions pkg/virtctl/create/preference/BUILD.bazel
Expand Up @@ -11,6 +11,7 @@ go_library(
"//vendor/github.com/spf13/cobra:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library",
"//vendor/k8s.io/client-go/tools/clientcmd:go_default_library",
"//vendor/sigs.k8s.io/yaml:go_default_library",
],
)
Expand Down
38 changes: 30 additions & 8 deletions pkg/virtctl/create/preference/preference.go
Expand Up @@ -21,11 +21,11 @@ package preference

import (
"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/util/rand"
"sigs.k8s.io/yaml"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/rand"
"k8s.io/client-go/tools/clientcmd"
instancetypev1beta1 "kubevirt.io/api/instancetype/v1beta1"
"sigs.k8s.io/yaml"

"kubevirt.io/kubevirt/pkg/virtctl/create/params"
)
Expand All @@ -46,11 +46,14 @@ const (
)

type createPreference struct {
namespace string
name string
namespaced bool
CPUTopology string
machineType string
preferredStorageClass string

clientConfig clientcmd.ClientConfig
}

type optionFn func(*createPreference, *instancetypev1beta1.VirtualMachinePreferenceSpec) error
Expand All @@ -61,8 +64,10 @@ var optFns = map[string]optionFn{
CPUTopologyFlag: withCPUTopology,
}

func NewCommand() *cobra.Command {
c := createPreference{}
func NewCommand(clientConfig clientcmd.ClientConfig) *cobra.Command {
c := createPreference{
clientConfig: clientConfig,
}
cmd := &cobra.Command{
Use: Preference,
Short: "Create a VirtualMachinePreference or VirtualMachineClusterPreference manifest.",
Expand All @@ -80,16 +85,27 @@ func NewCommand() *cobra.Command {
return cmd
}

func (c *createPreference) setDefaults(cmd *cobra.Command) {
func (c *createPreference) setDefaults(cmd *cobra.Command) error {
namespace, overridden, err := c.clientConfig.Namespace()
if err != nil {
return err
}
if overridden {
c.namespace = namespace
c.namespaced = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question like for VirtualMachineInstancetype.

}

if cmd.Flags().Changed(NameFlag) {
return
return nil
}

if c.namespaced {
c.name = "preference-" + rand.String(5)
} else {
c.name = "clusterpreference-" + rand.String(5)
}

return nil
}

func withVolumeStorageClass(c *createPreference, preferenceSpec *instancetypev1beta1.VirtualMachinePreferenceSpec) error {
Expand Down Expand Up @@ -147,7 +163,7 @@ func (c *createPreference) newClusterPreference() *instancetypev1beta1.VirtualMa
}

func (c *createPreference) newPreference() *instancetypev1beta1.VirtualMachinePreference {
return &instancetypev1beta1.VirtualMachinePreference{
preference := &instancetypev1beta1.VirtualMachinePreference{
TypeMeta: metav1.TypeMeta{
Kind: "VirtualMachinePreference",
APIVersion: instancetypev1beta1.SchemeGroupVersion.String(),
Expand All @@ -156,6 +172,12 @@ func (c *createPreference) newPreference() *instancetypev1beta1.VirtualMachinePr
Name: c.name,
},
}

if c.namespace != "" {
preference.Namespace = c.namespace
}

return preference
}

func (c *createPreference) applyFlags(cmd *cobra.Command, preferenceSpec *instancetypev1beta1.VirtualMachinePreferenceSpec) error {
Expand Down
15 changes: 15 additions & 0 deletions pkg/virtctl/create/preference/preference_test.go
Expand Up @@ -93,6 +93,21 @@ var _ = Describe("create", func() {
Entry("VirtualMachineClusterPreference", "", "preferThreads", false),
)

It("should create namespaced object and apply namespace when namespace is specified", func() {
const namespace = "my-namespace"
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(create, Preference,
setFlag("namespace", namespace),
)()
Expect(err).ToNot(HaveOccurred())

decodedObj, err := runtime.Decode(generatedscheme.Codecs.UniversalDeserializer(), bytes)
Expect(err).ToNot(HaveOccurred())

preference, ok := decodedObj.(*instancetypev1beta1.VirtualMachinePreference)
Expect(ok).To(BeTrue())
Expect(preference.Namespace).To(Equal(namespace))
})

DescribeTable("should fail with invalid CPU topology values", func(namespacedFlag, CPUTopology string, namespaced bool) {
err := clientcmd.NewRepeatableVirtctlCommand(create, Preference, namespacedFlag,
setFlag(CPUTopologyFlag, CPUTopology),
Expand Down
1 change: 1 addition & 0 deletions pkg/virtctl/create/vm/BUILD.bazel
Expand Up @@ -15,6 +15,7 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library",
"//vendor/k8s.io/client-go/tools/clientcmd:go_default_library",
"//vendor/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library",
"//vendor/sigs.k8s.io/yaml:go_default_library",
],
Expand Down