Skip to content

Commit

Permalink
Merge pull request kubevirt#12754 from 0xFelix/virtctl-create-prefere…
Browse files Browse the repository at this point in the history
…nce-tests

Drop virtctl create instancetype and preference functests and refactors unit tests
  • Loading branch information
kubevirt-bot authored Sep 11, 2024
2 parents a6b2dee + b9376c4 commit bfcdadb
Show file tree
Hide file tree
Showing 8 changed files with 247 additions and 431 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (f *ClusterPreferenceAdmitter) Admit(_ context.Context, ar *admissionv1.Adm
return admitPreference(ar.Request, instancetypeapi.ClusterPluralPreferenceResourceName)
}

func validatePreferenceSpec(field *k8sfield.Path, spec *instancetypeapiv1beta1.VirtualMachinePreferenceSpec) []metav1.StatusCause {
func ValidatePreferenceSpec(field *k8sfield.Path, spec *instancetypeapiv1beta1.VirtualMachinePreferenceSpec) []metav1.StatusCause {
var causes []metav1.StatusCause

causes = append(causes, validatePreferredCPUTopology(field, spec)...)
Expand Down Expand Up @@ -134,7 +134,7 @@ func admitPreference(request *admissionv1.AdmissionRequest, resource string) *ad
if err != nil {
return webhookutils.ToAdmissionResponseError(err)
}
causes := validatePreferenceSpec(k8sfield.NewPath("spec"), preferenceSpec)
causes := ValidatePreferenceSpec(k8sfield.NewPath("spec"), preferenceSpec)
if len(causes) > 0 {
return webhookutils.ToAdmissionResponse(causes)
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/virtctl/create/instancetype/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,19 @@ go_test(
],
deps = [
":go_default_library",
"//pkg/virt-api/webhooks/validating-webhook/admitters:go_default_library",
"//pkg/virtctl/create:go_default_library",
"//staging/src/kubevirt.io/api/core/v1:go_default_library",
"//staging/src/kubevirt.io/api/instancetype:go_default_library",
"//staging/src/kubevirt.io/api/instancetype/v1beta1:go_default_library",
"//staging/src/kubevirt.io/client-go/generated/kubevirt/clientset/versioned/scheme:go_default_library",
"//staging/src/kubevirt.io/client-go/testutils:go_default_library",
"//tests/clientcmd:go_default_library",
"//vendor/github.com/onsi/ginkgo/v2:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_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/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
],
)
250 changes: 140 additions & 110 deletions pkg/virtctl/create/instancetype/instancetype_test.go

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions pkg/virtctl/create/preference/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,17 @@ go_test(
],
deps = [
":go_default_library",
"//pkg/virt-api/webhooks/validating-webhook/admitters:go_default_library",
"//pkg/virtctl/create:go_default_library",
"//staging/src/kubevirt.io/api/instancetype:go_default_library",
"//staging/src/kubevirt.io/api/instancetype/v1beta1:go_default_library",
"//staging/src/kubevirt.io/client-go/generated/kubevirt/clientset/versioned/scheme:go_default_library",
"//staging/src/kubevirt.io/client-go/testutils:go_default_library",
"//tests/clientcmd:go_default_library",
"//vendor/github.com/onsi/ginkgo/v2:go_default_library",
"//vendor/github.com/onsi/gomega:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
],
)
168 changes: 95 additions & 73 deletions pkg/virtctl/create/preference/preference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,107 +20,125 @@ package preference_test

import (
"fmt"
"strings"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"

k8sv1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"

"kubevirt.io/api/instancetype"
instancetypev1beta1 "kubevirt.io/api/instancetype/v1beta1"
generatedscheme "kubevirt.io/client-go/generated/kubevirt/clientset/versioned/scheme"

"kubevirt.io/kubevirt/pkg/virt-api/webhooks/validating-webhook/admitters"
"kubevirt.io/kubevirt/pkg/virtctl/create"
. "kubevirt.io/kubevirt/pkg/virtctl/create/preference"
"kubevirt.io/kubevirt/tests/clientcmd"
)

const (
create = "create"
namespaced = "--namespaced"
)

var _ = Describe("create", func() {
Context("preference without arguments", func() {
DescribeTable("should succeed", func(namespacedFlag string, namespaced bool) {
err := clientcmd.NewRepeatableVirtctlCommand(create, Preference, namespacedFlag)()
Expect(err).ToNot(HaveOccurred())
var _ = Describe("create preference", func() {
DescribeTable("should fail with invalid CPU topology values", func(topology string, namespaced bool) {
args := []string{create.CREATE, Preference,
setFlag(CPUTopologyFlag, topology),
}
if namespaced {
args = append(args, setFlag(NamespacedFlag, "true"))
}
cmd := clientcmd.NewRepeatableVirtctlCommand(args...)
Expect(cmd()).To(MatchError(ContainSubstring(CPUTopologyErr)))
},
Entry("VirtualMachinePreference", "invalidCPU", true),
Entry("VirtualMachineClusterPreference", "clusterInvalidCPU", false),
)

Context("should succeed", func() {
DescribeTable("without arguments", func(namespaced bool) {
args := []string{create.CREATE, Preference}
if namespaced {
args = append(args, setFlag(NamespacedFlag, "true"))
}
cmd := clientcmd.NewRepeatableVirtctlCommand(args...)
Expect(cmd()).To(Succeed())
},
Entry("VirtualMachinePreference", namespaced, true),
Entry("VirtualMachineClusterPreference", "", false),
Entry("VirtualMachinePreference", true),
Entry("VirtualMachineClusterPreference", false),
)
})

Context("preference with arguments", func() {
var preferenceSpec *instancetypev1beta1.VirtualMachinePreferenceSpec

DescribeTable("should succeed with defined preferred storage class", func(namespacedFlag, PreferredstorageClass string, namespaced bool) {
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(create, Preference, namespacedFlag,
setFlag(VolumeStorageClassFlag, PreferredstorageClass),
)()
DescribeTable("with defined preferred storage class", func(storageClass string, namespaced bool) {
args := []string{create.CREATE, Preference,
setFlag(VolumeStorageClassFlag, storageClass),
}
if namespaced {
args = append(args, setFlag(NamespacedFlag, "true"))
}
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(args...)()
Expect(err).ToNot(HaveOccurred())

preferenceSpec, err = getPreferenceSpec(bytes, namespaced)
spec, err := getPreferenceSpec(bytes, namespaced)
Expect(err).ToNot(HaveOccurred())
Expect(preferenceSpec.Volumes.PreferredStorageClassName).To(Equal(PreferredstorageClass))
Expect(spec.Volumes.PreferredStorageClassName).To(Equal(storageClass))
Expect(validatePreferenceSpec(spec)).To(BeEmpty())
},
Entry("VirtualMachinePreference", namespaced, "testing-storage", true),
Entry("VirtualMachineClusterPreference", "", "hostpath-provisioner", false),
Entry("VirtualMachinePreference", "testing-storage", true),
Entry("VirtualMachineClusterPreference", "hostpath-provisioner", false),
)

DescribeTable("should succeed with defined machine type", func(namespacedFlag, machineType string, namespaced bool) {
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(create, Preference, namespacedFlag,
DescribeTable("with defined machine type", func(machineType string, namespaced bool) {
args := []string{create.CREATE, Preference,
setFlag(MachineTypeFlag, machineType),
)()
}
if namespaced {
args = append(args, setFlag(NamespacedFlag, "true"))
}
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(args...)()
Expect(err).ToNot(HaveOccurred())

preferenceSpec, err = getPreferenceSpec(bytes, namespaced)
spec, err := getPreferenceSpec(bytes, namespaced)
Expect(err).ToNot(HaveOccurred())
Expect(preferenceSpec.Machine.PreferredMachineType).To(Equal(machineType))
Expect(spec.Machine.PreferredMachineType).To(Equal(machineType))
Expect(validatePreferenceSpec(spec)).To(BeEmpty())
},
Entry("VirtualMachinePreference", namespaced, "pc-i440fx-2.10", true),
Entry("VirtualMachineClusterPreference", "", "pc-q35-2.10", false),
Entry("VirtualMachinePreference", "pc-i440fx-2.10", true),
Entry("VirtualMachineClusterPreference", "pc-q35-2.10", false),
)

DescribeTable("should succeed with defined CPU topology", func(namespacedFlag, CPUTopology string, namespaced bool) {
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(create, Preference, namespacedFlag,
setFlag(CPUTopologyFlag, CPUTopology),
)()
DescribeTable("with defined CPU topology", func(topology instancetypev1beta1.PreferredCPUTopology, namespaced bool) {
args := []string{create.CREATE, Preference,
setFlag(CPUTopologyFlag, string(topology)),
}
if namespaced {
args = append(args, setFlag(NamespacedFlag, "true"))
}
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(args...)()
Expect(err).ToNot(HaveOccurred())

preferenceSpec, err = getPreferenceSpec(bytes, namespaced)
spec, err := getPreferenceSpec(bytes, namespaced)
Expect(err).ToNot(HaveOccurred())
Expect(preferenceSpec.CPU.PreferredCPUTopology).ToNot(BeNil())
Expect(*preferenceSpec.CPU.PreferredCPUTopology).To(Equal(instancetypev1beta1.PreferredCPUTopology(CPUTopology)))
Expect(spec.CPU.PreferredCPUTopology).ToNot(BeNil())
Expect(*spec.CPU.PreferredCPUTopology).To(Equal(topology))
Expect(validatePreferenceSpec(spec)).To(BeEmpty())
},
Entry("VirtualMachinePreference", namespaced, "preferCores", true),
Entry("VirtualMachineClusterPreference", "", "preferThreads", false),
Entry("VirtualMachinePreference", instancetypev1beta1.DeprecatedPreferCores, true),
Entry("VirtualMachineClusterPreference", instancetypev1beta1.DeprecatedPreferThreads, 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))
})
It("should create namespaced object and apply namespace when namespace is specified", func() {
const namespace = "my-namespace"
bytes, err := clientcmd.NewRepeatableVirtctlCommandWithOut(create.CREATE, Preference,
setFlag("namespace", namespace),
)()
Expect(err).ToNot(HaveOccurred())

DescribeTable("should fail with invalid CPU topology values", func(namespacedFlag, CPUTopology string, namespaced bool) {
err := clientcmd.NewRepeatableVirtctlCommand(create, Preference, namespacedFlag,
setFlag(CPUTopologyFlag, CPUTopology),
)()

Expect(err).To(HaveOccurred())
Expect(err.Error()).To(ContainSubstring(CPUTopologyFlag))
Expect(err.Error()).To(ContainSubstring(CPUTopologyErr))
},
Entry("VirtualMachinePreference", namespaced, "invalidCPU", true),
Entry("VirtualMachineClusterPreference", "", "clusterInvalidCPU", false),
)
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))
})
})

Expand All @@ -130,18 +148,22 @@ func setFlag(flag, parameter string) string {

func getPreferenceSpec(bytes []byte, namespaced bool) (*instancetypev1beta1.VirtualMachinePreferenceSpec, error) {
decodedObj, err := runtime.Decode(generatedscheme.Codecs.UniversalDeserializer(), bytes)
ExpectWithOffset(1, err).ToNot(HaveOccurred())
Expect(err).ToNot(HaveOccurred())

switch obj := decodedObj.(type) {
case *instancetypev1beta1.VirtualMachinePreference:
ExpectWithOffset(1, namespaced).To(BeTrue(), "expected VirtualMachinePreference to be created")
ExpectWithOffset(1, obj.Kind).To(Equal("VirtualMachinePreference"))
Expect(namespaced).To(BeTrue())
Expect(strings.ToLower(obj.Kind)).To(Equal(instancetype.SingularPreferenceResourceName))
return &obj.Spec, nil
case *instancetypev1beta1.VirtualMachineClusterPreference:
ExpectWithOffset(1, namespaced).To(BeFalse(), "expected VirtualMachineClusterPreference to be created")
ExpectWithOffset(1, obj.Kind).To(Equal("VirtualMachineClusterPreference"))
Expect(namespaced).To(BeFalse())
Expect(strings.ToLower(obj.Kind)).To(Equal(instancetype.ClusterSingularPreferenceResourceName))
return &obj.Spec, nil
default:
return nil, fmt.Errorf("object must be VirtualMachinePreference or VirtualMachineClusterPreference")
}

return nil, fmt.Errorf("object must be VirtualMachinePreference or VirtualMachineClusterPreference")
}

func validatePreferenceSpec(spec *instancetypev1beta1.VirtualMachinePreferenceSpec) []k8sv1.StatusCause {
return admitters.ValidatePreferenceSpec(field.NewPath("spec"), spec)
}
7 changes: 0 additions & 7 deletions tests/virtctl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library")
go_library(
name = "go_default_library",
srcs = [
"create_instancetype.go",
"create_preference.go",
"create_vm.go",
"credentials.go",
"scp.go",
Expand All @@ -15,19 +13,15 @@ go_library(
deps = [
"//pkg/libvmi:go_default_library",
"//pkg/libvmi/cloudinit:go_default_library",
"//pkg/virtctl/create/instancetype:go_default_library",
"//pkg/virtctl/create/preference:go_default_library",
"//pkg/virtctl/create/vm:go_default_library",
"//staging/src/kubevirt.io/api/core/v1:go_default_library",
"//staging/src/kubevirt.io/api/instancetype:go_default_library",
"//staging/src/kubevirt.io/api/instancetype/v1beta1:go_default_library",
"//staging/src/kubevirt.io/client-go/generated/kubevirt/clientset/versioned/scheme:go_default_library",
"//staging/src/kubevirt.io/client-go/kubecli:go_default_library",
"//tests/clientcmd:go_default_library",
"//tests/console:go_default_library",
"//tests/containerdisk:go_default_library",
"//tests/decorators:go_default_library",
"//tests/framework/cleanup:go_default_library",
"//tests/framework/kubevirt:go_default_library",
"//tests/framework/matcher:go_default_library",
"//tests/libsecret:go_default_library",
Expand All @@ -43,7 +37,6 @@ go_library(
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_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/runtime:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/rand:go_default_library",
"//vendor/k8s.io/utils/pointer:go_default_library",
"//vendor/kubevirt.io/containerized-data-importer-api/pkg/apis/core/v1beta1:go_default_library",
Expand Down
Loading

0 comments on commit bfcdadb

Please sign in to comment.