Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
openSUSE:Leap:15.5:Update
kubevirt
0001-guestfs-flag-to-set-uid-and-gid.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0001-guestfs-flag-to-set-uid-and-gid.patch of Package kubevirt
From d1453961bccb953943133dbe510326b4991fa7b0 Mon Sep 17 00:00:00 2001 From: Vasiliy Ulyanov <vulyanov@suse.de> Date: Mon, 17 Oct 2022 14:17:00 +0200 Subject: [PATCH 01/12] Drop RunAsGroup from the libguestfs pod manifest Same as 0821a5c900. Specifying RunAsGroup without RunAsUser violates the specification: run_as_group should only be specified when run_as_user is specified; otherwise, the runtime MUST error. Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de> --- cmd/libguestfs/BUILD.bazel | 1 + pkg/virtctl/guestfs/guestfs.go | 13 ++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cmd/libguestfs/BUILD.bazel b/cmd/libguestfs/BUILD.bazel index 9f98e4f9c..0e850606c 100644 --- a/cmd/libguestfs/BUILD.bazel +++ b/cmd/libguestfs/BUILD.bazel @@ -27,6 +27,7 @@ container_image( "//:get-version", ], tars = [ + "//:passwd-tar", "//rpm:libguestfs-tools", ":appliance_layer", ":entrypoint", diff --git a/pkg/virtctl/guestfs/guestfs.go b/pkg/virtctl/guestfs/guestfs.go index d79d85db0..09a0c61b0 100644 --- a/pkg/virtctl/guestfs/guestfs.go +++ b/pkg/virtctl/guestfs/guestfs.go @@ -53,7 +53,7 @@ var ( kvm bool podName string root bool - group string + fsGroup string ) type guestfsCommand struct { @@ -78,7 +78,7 @@ func NewGuestfsShellCommand(clientConfig clientcmd.ClientConfig) *cobra.Command cmd.PersistentFlags().BoolVar(&kvm, "kvm", true, "Use kvm for the libguestfs-tools container") cmd.PersistentFlags().BoolVar(&root, "root", false, "Set uid 0 for the libguestfs-tool container") cmd.SetUsageTemplate(templates.UsageTemplate()) - cmd.PersistentFlags().StringVar(&group, "group", "", "Set the gid and fsgroup for the libguestfs-tool container") + cmd.PersistentFlags().StringVar(&fsGroup, "fsGroup", "", "Set the fsgroup for the libguestfs-tool container") return cmd } @@ -369,11 +369,11 @@ func (client *K8sClient) getPodsForPVC(pvcName, ns string) ([]corev1.Pod, error) } func setGroupLibguestfs() (*int64, error) { - if root && group != "" { - return nil, fmt.Errorf("cannot set group id with root") + if root && fsGroup != "" { + return nil, fmt.Errorf("cannot set fsGroup id with root") } - if group != "" { - n, err := strconv.ParseInt(group, 10, 64) + if fsGroup != "" { + n, err := strconv.ParseInt(fsGroup, 10, 64) if err != nil { return nil, err } @@ -418,7 +418,6 @@ func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock boo securityContext := &corev1.PodSecurityContext{ RunAsNonRoot: &nonRoot, RunAsUser: uid, - RunAsGroup: g, FSGroup: g, SeccompProfile: &corev1.SeccompProfile{ Type: corev1.SeccompProfileTypeRuntimeDefault, -- 2.38.1 From 0335eebaa330984e00bc49eaf2a243e0f73c3fbe Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Mon, 22 Aug 2022 08:31:59 +0200 Subject: [PATCH 02/12] allow user to set the pod uid Add a new flag uid if the cluster defaults don't work. This flag enables the users to specify the uid for running the pod. Signed-off-by: Alice Frosi <afrosi@redhat.com> --- pkg/virtctl/guestfs/BUILD.bazel | 1 + pkg/virtctl/guestfs/guestfs.go | 38 +++++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/pkg/virtctl/guestfs/BUILD.bazel b/pkg/virtctl/guestfs/BUILD.bazel index 0ee11f549..ee9a66354 100644 --- a/pkg/virtctl/guestfs/BUILD.bazel +++ b/pkg/virtctl/guestfs/BUILD.bazel @@ -18,6 +18,7 @@ go_library( "//vendor/k8s.io/client-go/rest:go_default_library", "//vendor/k8s.io/client-go/tools/clientcmd:go_default_library", "//vendor/k8s.io/client-go/tools/remotecommand:go_default_library", + "//vendor/k8s.io/utils/pointer:go_default_library", ], ) diff --git a/pkg/virtctl/guestfs/guestfs.go b/pkg/virtctl/guestfs/guestfs.go index 09a0c61b0..23cec45e4 100644 --- a/pkg/virtctl/guestfs/guestfs.go +++ b/pkg/virtctl/guestfs/guestfs.go @@ -22,6 +22,8 @@ import ( "kubevirt.io/client-go/kubecli" + "k8s.io/utils/pointer" + "kubevirt.io/kubevirt/pkg/virtctl/templates" "kubevirt.io/kubevirt/pkg/virtctl/utils" ) @@ -54,6 +56,7 @@ var ( podName string root bool fsGroup string + uid string ) type guestfsCommand struct { @@ -77,6 +80,7 @@ func NewGuestfsShellCommand(clientConfig clientcmd.ClientConfig) *cobra.Command cmd.PersistentFlags().StringVar(&pullPolicy, "pull-policy", string(pullPolicyDefault), "pull policy for the libguestfs image") cmd.PersistentFlags().BoolVar(&kvm, "kvm", true, "Use kvm for the libguestfs-tools container") cmd.PersistentFlags().BoolVar(&root, "root", false, "Set uid 0 for the libguestfs-tool container") + cmd.PersistentFlags().StringVar(&uid, "uid", "", "Set uid for the libguestfs-tool container. It doesn't work with root") cmd.SetUsageTemplate(templates.UsageTemplate()) cmd.PersistentFlags().StringVar(&fsGroup, "fsGroup", "", "Set the fsgroup for the libguestfs-tool container") @@ -386,6 +390,26 @@ func setGroupLibguestfs() (*int64, error) { return nil, nil } +// setUIDLibugestfs returns the guestfs uid +func setUIDLibugestfs() (*int64, error) { + switch { + case root: + var zero int64 + if uid != "" { + return nil, fmt.Errorf("cannot set uid if root is true") + } + return &zero, nil + case uid != "": + n, err := strconv.ParseInt(uid, 10, 64) + if err != nil { + return nil, err + } + return &n, nil + default: + return nil, nil + } +} + func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock bool) (*corev1.Pod, error) { var resources corev1.ResourceRequirements podName = fmt.Sprintf("%s-%s", podNamePrefix, pvc) @@ -396,12 +420,9 @@ func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock boo }, } } - nonRoot := true - var uidRoot int64 = 0 - var uid *int64 - if root { - nonRoot = false - uid = &uidRoot + u, err := setUIDLibugestfs() + if err != nil { + return nil, err } g, err := setGroupLibguestfs() if err != nil { @@ -414,10 +435,9 @@ func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock boo Drop: []corev1.Capability{"ALL"}, }, } - securityContext := &corev1.PodSecurityContext{ - RunAsNonRoot: &nonRoot, - RunAsUser: uid, + RunAsNonRoot: pointer.Bool(!root), + RunAsUser: u, FSGroup: g, SeccompProfile: &corev1.SeccompProfile{ Type: corev1.SeccompProfileTypeRuntimeDefault, -- 2.38.1 From 1ebc15d9785f4cf8d6f757095424adeca528f15c Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Tue, 23 Aug 2022 11:32:00 +0200 Subject: [PATCH 03/12] add unit test Verify that root and uid flags cannot be used together Signed-off-by: Alice Frosi <afrosi@redhat.com> --- pkg/virtctl/guestfs/guestfs_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pkg/virtctl/guestfs/guestfs_test.go b/pkg/virtctl/guestfs/guestfs_test.go index 5ceb10476..0990e7cd2 100644 --- a/pkg/virtctl/guestfs/guestfs_test.go +++ b/pkg/virtctl/guestfs/guestfs_test.go @@ -148,6 +148,14 @@ var _ = Describe("Guestfs shell", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(Equal(fmt.Sprintf("The PVC %s doesn't exist", pvcName))) }) + + It("UID cannot be used with root", func() { + guestfs.SetClient(fakeCreateClientPVC) + cmd := virtctlcmd.NewRepeatableVirtctlCommand(commandName, pvcName, "--root=true", "--uid=1001") + err := cmd() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(Equal(fmt.Sprintf("cannot set uid if root is true"))) + }) }) Context("URL authenticity", func() { -- 2.38.1 From a72e5ff9ee553e8074cc413787f32f68445be677 Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Tue, 23 Aug 2022 11:35:53 +0200 Subject: [PATCH 04/12] add functional test Verify that guestfs command can be run under another uid Signed-off-by: Alice Frosi <afrosi@redhat.com> --- tests/storage/guestfs.go | 41 ++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/tests/storage/guestfs.go b/tests/storage/guestfs.go index fcb19cb61..acdb1a509 100644 --- a/tests/storage/guestfs.go +++ b/tests/storage/guestfs.go @@ -107,6 +107,18 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { } + verifyCanRunOnFSPVC := func(podName string) { + stdout, stderr, err := execCommandLibguestfsPod(podName, []string{"qemu-img", "create", "/disk/disk.img", "500M"}) + Expect(stderr).To(Equal("")) + Expect(stdout).To(ContainSubstring("Formatting")) + Expect(err).ToNot(HaveOccurred()) + stdout, stderr, err = execCommandLibguestfsPod(podName, []string{"guestfish", "-a", "/disk/disk.img", "run"}) + Expect(stderr).To(BeEmpty()) + Expect(stdout).To(BeEmpty()) + Expect(err).ToNot(HaveOccurred()) + + } + Context("Run libguestfs on PVCs", func() { BeforeEach(func() { var err error @@ -142,15 +154,7 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { podName := libguestsTools + pvcClaim createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim) - stdout, stderr, err := execCommandLibguestfsPod(podName, []string{"qemu-img", "create", "/disk/disk.img", "500M"}) - Expect(stderr).To(Equal("")) - Expect(stdout).To(ContainSubstring("Formatting")) - Expect(err).ToNot(HaveOccurred()) - stdout, stderr, err = execCommandLibguestfsPod(podName, []string{"guestfish", "-a", "/disk/disk.img", "run"}) - Expect(stderr).To(Equal("")) - Expect(stdout).To(Equal("")) - Expect(err).ToNot(HaveOccurred()) - + verifyCanRunOnFSPVC(podName) }) It("[posneg:negative][test_id:6480]Should fail to run the guestfs command on a PVC in use", func() { @@ -179,6 +183,15 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { Expect(err).ToNot(HaveOccurred()) }) + It("Should successfully run guestfs command on a filesystem-based PVC setting the uid", func() { + f := createFakeAttacher() + defer f.closeChannel() + pvcClaim = "pvc-fs-with-different-uid" + podName := libguestsTools + pvcClaim + createPVCFilesystem(pvcClaim) + runGuestfsOnPVC(pvcClaim, "--uid", "1002") + verifyCanRunOnFSPVC(podName) + }) }) Context("Run libguestfs on PVCs with root", func() { BeforeEach(func() { @@ -199,15 +212,7 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { podName := libguestsTools + pvcClaim createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim, "--root") - stdout, stderr, err := execCommandLibguestfsPod(podName, []string{"qemu-img", "create", "/disk/disk.img", "500M"}) - Expect(stderr).To(Equal("")) - Expect(stdout).To(ContainSubstring("Formatting")) - Expect(err).ToNot(HaveOccurred()) - stdout, stderr, err = execCommandLibguestfsPod(podName, []string{"guestfish", "-a", "/disk/disk.img", "run"}) - Expect(stderr).To(Equal("")) - Expect(stdout).To(Equal("")) - Expect(err).ToNot(HaveOccurred()) - + verifyCanRunOnFSPVC(podName) }) }) -- 2.38.1 From 6b7dd4c6a4773c5240810d929e5bedf10a62601b Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Thu, 20 Oct 2022 10:50:03 +0200 Subject: [PATCH 05/12] adjust variables/function names Rename functions and variables to match with the fsgroup. Signed-off-by: Alice Frosi <afrosi@redhat.com> --- pkg/virtctl/guestfs/guestfs.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/virtctl/guestfs/guestfs.go b/pkg/virtctl/guestfs/guestfs.go index 23cec45e4..bac418409 100644 --- a/pkg/virtctl/guestfs/guestfs.go +++ b/pkg/virtctl/guestfs/guestfs.go @@ -372,7 +372,7 @@ func (client *K8sClient) getPodsForPVC(pvcName, ns string) ([]corev1.Pod, error) return pods, nil } -func setGroupLibguestfs() (*int64, error) { +func setFSGroupLibguestfs() (*int64, error) { if root && fsGroup != "" { return nil, fmt.Errorf("cannot set fsGroup id with root") } @@ -384,8 +384,8 @@ func setGroupLibguestfs() (*int64, error) { return &n, nil } if root { - var rootGID int64 = 0 - return &rootGID, nil + var rootFsID int64 = 0 + return &rootFsID, nil } return nil, nil } @@ -424,7 +424,7 @@ func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock boo if err != nil { return nil, err } - g, err := setGroupLibguestfs() + f, err := setFSGroupLibguestfs() if err != nil { return nil, err } @@ -438,7 +438,7 @@ func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock boo securityContext := &corev1.PodSecurityContext{ RunAsNonRoot: pointer.Bool(!root), RunAsUser: u, - FSGroup: g, + FSGroup: f, SeccompProfile: &corev1.SeccompProfile{ Type: corev1.SeccompProfileTypeRuntimeDefault, }, -- 2.38.1 From e7a5056ba3b83cfd18991f8b801a977becc635fb Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Thu, 20 Oct 2022 10:37:13 +0200 Subject: [PATCH 06/12] guestfs: add gid flag The gid flags set the group for the container. This flag can be used only combined wit the uid flag. See comment: https://github.com/kubernetes/cri-api/blob/2b5244cefaeace624cb160d6b3d85dd3fd14baea/pkg/apis/runtime/v1/api.proto#L307-L309 Signed-off-by: Alice Frosi <afrosi@redhat.com> --- pkg/virtctl/guestfs/guestfs.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pkg/virtctl/guestfs/guestfs.go b/pkg/virtctl/guestfs/guestfs.go index bac418409..3c757b6ad 100644 --- a/pkg/virtctl/guestfs/guestfs.go +++ b/pkg/virtctl/guestfs/guestfs.go @@ -57,6 +57,7 @@ var ( root bool fsGroup string uid string + gid string ) type guestfsCommand struct { @@ -81,6 +82,7 @@ func NewGuestfsShellCommand(clientConfig clientcmd.ClientConfig) *cobra.Command cmd.PersistentFlags().BoolVar(&kvm, "kvm", true, "Use kvm for the libguestfs-tools container") cmd.PersistentFlags().BoolVar(&root, "root", false, "Set uid 0 for the libguestfs-tool container") cmd.PersistentFlags().StringVar(&uid, "uid", "", "Set uid for the libguestfs-tool container. It doesn't work with root") + cmd.PersistentFlags().StringVar(&gid, "gid", "", "Set gid for the libguestfs-tool container. This works only combined when the uid is manually set") cmd.SetUsageTemplate(templates.UsageTemplate()) cmd.PersistentFlags().StringVar(&fsGroup, "fsGroup", "", "Set the fsgroup for the libguestfs-tool container") @@ -410,6 +412,29 @@ func setUIDLibugestfs() (*int64, error) { } } +func setGIDLibguestfs() (*int64, error) { + // The GID can only be specified together with the uid. See comment at: https://github.com/kubernetes/cri-api/blob/2b5244cefaeace624cb160d6b3d85dd3fd14baea/pkg/apis/runtime/v1/api.proto#L307-L309 + if gid != "" && uid == "" { + return nil, fmt.Errorf("gid requires the uid to be set") + } + + if root && gid != "" { + return nil, fmt.Errorf("cannot set gid id with root") + } + if gid != "" { + n, err := strconv.ParseInt(gid, 10, 64) + if err != nil { + return nil, err + } + return &n, nil + } + if root { + var rootGID int64 = 0 + return &rootGID, nil + } + return nil, nil +} + func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock bool) (*corev1.Pod, error) { var resources corev1.ResourceRequirements podName = fmt.Sprintf("%s-%s", podNamePrefix, pvc) @@ -424,6 +449,10 @@ func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock boo if err != nil { return nil, err } + g, err := setGIDLibguestfs() + if err != nil { + return nil, err + } f, err := setFSGroupLibguestfs() if err != nil { return nil, err @@ -438,6 +467,7 @@ func createLibguestfsPod(pvc, image, cmd string, args []string, kvm, isBlock boo securityContext := &corev1.PodSecurityContext{ RunAsNonRoot: pointer.Bool(!root), RunAsUser: u, + RunAsGroup: g, FSGroup: f, SeccompProfile: &corev1.SeccompProfile{ Type: corev1.SeccompProfileTypeRuntimeDefault, -- 2.38.1 From c77ccabeabffcb99bc84da854696046f68eb4acc Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Thu, 20 Oct 2022 11:19:42 +0200 Subject: [PATCH 07/12] add unit test for gid Verify that the gid can be only used together with the uid. Signed-off-by: Alice Frosi <afrosi@redhat.com> --- pkg/virtctl/guestfs/guestfs_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/virtctl/guestfs/guestfs_test.go b/pkg/virtctl/guestfs/guestfs_test.go index 0990e7cd2..dcce3f71d 100644 --- a/pkg/virtctl/guestfs/guestfs_test.go +++ b/pkg/virtctl/guestfs/guestfs_test.go @@ -156,6 +156,13 @@ var _ = Describe("Guestfs shell", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).Should(Equal(fmt.Sprintf("cannot set uid if root is true"))) }) + It("GID can be use only together with the uid flag", func() { + guestfs.SetClient(fakeCreateClientPVC) + cmd := virtctlcmd.NewRepeatableVirtctlCommand(commandName, pvcName, "--gid=1001") + err := cmd() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).Should(Equal(fmt.Sprintf("gid requires the uid to be set"))) + }) }) Context("URL authenticity", func() { -- 2.38.1 From 62bc70cf34751f19fc4a19dc66ee46c56172d802 Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Thu, 20 Oct 2022 16:01:01 +0200 Subject: [PATCH 08/12] fix fsgrouo flag in the test case Change flag from group to fsGroup. Signed-off-by: Alice Frosi <afrosi@redhat.com> --- tests/storage/guestfs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storage/guestfs.go b/tests/storage/guestfs.go index acdb1a509..91d36fbe7 100644 --- a/tests/storage/guestfs.go +++ b/tests/storage/guestfs.go @@ -77,7 +77,7 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { podName := libguestsTools + pvcClaim o := append([]string{"guestfs", pvcClaim, "--namespace", util.NamespaceTestDefault}, options...) if setGroup { - o = append(o, "--group", testGroup) + o = append(o, "--fsGroup", testGroup) } guestfsCmd := clientcmd.NewVirtctlCommand(o...) go func() { -- 2.38.1 From d8b43890688deb3f92d2325b31b08a03a32827f2 Mon Sep 17 00:00:00 2001 From: Maya Rashish <mrashish@redhat.com> Date: Wed, 9 Nov 2022 08:45:09 +0200 Subject: [PATCH 09/12] Use a unique pvc/pod name for guestfs test We have a flake where the pod is terminating -- this is likely due to the name conflict with another test. This name is more appropriate, too. Signed-off-by: Maya Rashish <mrashish@redhat.com> --- tests/storage/guestfs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storage/guestfs.go b/tests/storage/guestfs.go index 91d36fbe7..5e5d40d2c 100644 --- a/tests/storage/guestfs.go +++ b/tests/storage/guestfs.go @@ -208,7 +208,7 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { It("Should successfully run guestfs command on a filesystem-based PVC with root", func() { f := createFakeAttacher() defer f.closeChannel() - pvcClaim = "pvc-fs-with-different-uid" + pvcClaim = "pvc-fs-with-root" podName := libguestsTools + pvcClaim createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim, "--root") -- 2.38.1 From 433d92489b305048d1fe5d9c297b5a23f887ad42 Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Fri, 11 Nov 2022 09:50:50 +0100 Subject: [PATCH 10/12] Get guestfs pod name with getGuestfsPodName Signed-off-by: Alice Frosi <afrosi@redhat.com> --- tests/storage/guestfs.go | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/tests/storage/guestfs.go b/tests/storage/guestfs.go index 5e5d40d2c..022bd0ace 100644 --- a/tests/storage/guestfs.go +++ b/tests/storage/guestfs.go @@ -20,8 +20,6 @@ import ( "kubevirt.io/kubevirt/tests/util" ) -const libguestsTools = "libguestfs-tools-" - type fakeAttacher struct { done chan bool } @@ -43,6 +41,11 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { setGroup bool testGroup string ) + + getGuestfsPodName := func(pvc string) string { + return "libguestfs-tools-" + pvc + } + execCommandLibguestfsPod := func(podName string, c []string) (string, string, error) { pod, err := virtClient.CoreV1().Pods(util.NamespaceTestDefault).Get(context.Background(), podName, metav1.GetOptions{}) Expect(err).ToNot(HaveOccurred()) @@ -74,7 +77,7 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { } runGuestfsOnPVC := func(pvcClaim string, options ...string) { - podName := libguestsTools + pvcClaim + podName := getGuestfsPodName(pvcClaim) o := append([]string{"guestfs", pvcClaim, "--namespace", util.NamespaceTestDefault}, options...) if setGroup { o = append(o, "--fsGroup", testGroup) @@ -141,7 +144,7 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { pvcClaim = "pvc-verify" createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim) - output, _, err := execCommandLibguestfsPod(libguestsTools+pvcClaim, []string{"libguestfs-test-tool"}) + output, _, err := execCommandLibguestfsPod(getGuestfsPodName(pvcClaim), []string{"libguestfs-test-tool"}) Expect(err).ToNot(HaveOccurred()) Expect(output).To(ContainSubstring("===== TEST FINISHED OK =====")) @@ -151,10 +154,9 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { f := createFakeAttacher() defer f.closeChannel() pvcClaim = "pvc-fs" - podName := libguestsTools + pvcClaim createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim) - verifyCanRunOnFSPVC(podName) + verifyCanRunOnFSPVC(getGuestfsPodName(pvcClaim)) }) It("[posneg:negative][test_id:6480]Should fail to run the guestfs command on a PVC in use", func() { @@ -174,10 +176,9 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { defer f.closeChannel() pvcClaim = "pvc-block" - podName := libguestsTools + pvcClaim libstorage.CreateBlockPVC(pvcClaim, "500Mi") runGuestfsOnPVC(pvcClaim) - stdout, stderr, err := execCommandLibguestfsPod(podName, []string{"guestfish", "-a", "/dev/vda", "run"}) + stdout, stderr, err := execCommandLibguestfsPod(getGuestfsPodName(pvcClaim), []string{"guestfish", "-a", "/dev/vda", "run"}) Expect(stderr).To(Equal("")) Expect(stdout).To(Equal("")) Expect(err).ToNot(HaveOccurred()) @@ -187,10 +188,9 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { f := createFakeAttacher() defer f.closeChannel() pvcClaim = "pvc-fs-with-different-uid" - podName := libguestsTools + pvcClaim createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim, "--uid", "1002") - verifyCanRunOnFSPVC(podName) + verifyCanRunOnFSPVC(getGuestfsPodName(pvcClaim)) }) }) Context("Run libguestfs on PVCs with root", func() { @@ -209,10 +209,9 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { f := createFakeAttacher() defer f.closeChannel() pvcClaim = "pvc-fs-with-root" - podName := libguestsTools + pvcClaim createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim, "--root") - verifyCanRunOnFSPVC(podName) + verifyCanRunOnFSPVC(getGuestfsPodName(pvcClaim)) }) }) -- 2.38.1 From 70822e28f08fa399a514f1814d9433755b76f369 Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Mon, 14 Nov 2022 16:30:31 +0100 Subject: [PATCH 11/12] Set setGroup also for the second guestfs pod Signed-off-by: Alice Frosi <afrosi@redhat.com> --- tests/storage/guestfs.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/storage/guestfs.go b/tests/storage/guestfs.go index 022bd0ace..e1245ba7d 100644 --- a/tests/storage/guestfs.go +++ b/tests/storage/guestfs.go @@ -165,9 +165,13 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { pvcClaim = "pvc-fail-to-run-twice" createPVCFilesystem(pvcClaim) runGuestfsOnPVC(pvcClaim) - guestfsCmd := clientcmd.NewVirtctlCommand("guestfs", + options := []string{"guestfs", pvcClaim, - "--namespace", util.NamespaceTestDefault) + "--namespace", util.NamespaceTestDefault} + if setGroup { + options = append(options, "--fsGroup", testGroup) + } + guestfsCmd := clientcmd.NewVirtctlCommand(options...) Expect(guestfsCmd.Execute()).To(HaveOccurred()) }) -- 2.38.1 From c232a312b02685e1081326dbb8e834faee1b0b4f Mon Sep 17 00:00:00 2001 From: Alice Frosi <afrosi@redhat.com> Date: Wed, 9 Nov 2022 17:53:35 +0100 Subject: [PATCH 12/12] Solve flakiness for guestfs tests Sometimes, the libguestfs pod is deleted by the test clean-up but the function runGuestfsOnPVC hasn't gracefully terminated yet. The function runGuestfsOnPVC has a goroutine that returns when the channel of the fakeCreateAttacher is closed. With the new refactoring, a new channel has been added to select between an error in the guestfs routine and to gracefully terminates the test. Signed-off-by: Alice Frosi <afrosi@redhat.com> --- tests/storage/BUILD.bazel | 1 + tests/storage/guestfs.go | 69 +++++++++++++++++++++------------------ 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/tests/storage/BUILD.bazel b/tests/storage/BUILD.bazel index 76fff5109..fd4efd846 100644 --- a/tests/storage/BUILD.bazel +++ b/tests/storage/BUILD.bazel @@ -61,6 +61,7 @@ go_library( "//vendor/github.com/onsi/gomega/types:go_default_library", "//vendor/github.com/openshift/api/route/v1:go_default_library", "//vendor/github.com/pborman/uuid:go_default_library", + "//vendor/github.com/spf13/cobra:go_default_library", "//vendor/k8s.io/api/admissionregistration/v1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", "//vendor/k8s.io/api/networking/v1:go_default_library", diff --git a/tests/storage/guestfs.go b/tests/storage/guestfs.go index e1245ba7d..971c87822 100644 --- a/tests/storage/guestfs.go +++ b/tests/storage/guestfs.go @@ -6,6 +6,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -21,17 +22,20 @@ import ( ) type fakeAttacher struct { - done chan bool + // Channel to unblock the fake attacher for the console + doneAttacher chan bool + // Channel to unblock the goroutine for the guestfs command + doneGuestfs chan bool } // fakeCreateAttacher simulates the attacher to the pod console. It has to block until the test terminates. func (f *fakeAttacher) fakeCreateAttacher(client *guestfs.K8sClient, p *corev1.Pod, command string) error { - <-f.done + <-f.doneAttacher return nil } func (f *fakeAttacher) closeChannel() { - f.done <- true + f.doneGuestfs <- true } var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { @@ -71,22 +75,35 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { createFakeAttacher := func() *fakeAttacher { f := &fakeAttacher{} - f.done = make(chan bool, 1) + f.doneAttacher = make(chan bool, 1) + f.doneGuestfs = make(chan bool, 1) guestfs.SetAttacher(f.fakeCreateAttacher) return f } - runGuestfsOnPVC := func(pvcClaim string, options ...string) { + guestfsWithSync := func(f *fakeAttacher, guestfsCmd *cobra.Command) { + defer GinkgoRecover() + errChan := make(chan error) + go func() { + errChan <- guestfsCmd.Execute() + }() + select { + case <-f.doneGuestfs: + case err := <-errChan: + Expect(err).ToNot(HaveOccurred()) + } + // Unblock the fake attacher + f.doneAttacher <- true + } + + runGuestfsOnPVC := func(f *fakeAttacher, pvcClaim string, options ...string) { podName := getGuestfsPodName(pvcClaim) o := append([]string{"guestfs", pvcClaim, "--namespace", util.NamespaceTestDefault}, options...) if setGroup { o = append(o, "--fsGroup", testGroup) } guestfsCmd := clientcmd.NewVirtctlCommand(o...) - go func() { - defer GinkgoRecover() - Expect(guestfsCmd.Execute()).ToNot(HaveOccurred()) - }() + go guestfsWithSync(f, guestfsCmd) // Waiting until the libguestfs pod is running Eventually(func() bool { pod, _ := virtClient.CoreV1().Pods(util.NamespaceTestDefault).Get(context.Background(), podName, metav1.GetOptions{}) @@ -123,6 +140,7 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { } Context("Run libguestfs on PVCs", func() { + var f *fakeAttacher BeforeEach(func() { var err error virtClient, err = kubecli.GetKubevirtClient() @@ -130,41 +148,34 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { // TODO: Always setGroup to true until we have the ability to control how virtctl guestfs is run setGroup = true testGroup = "2000" + f = createFakeAttacher() }) AfterEach(func() { - err := virtClient.CoreV1().PersistentVolumeClaims(util.NamespaceTestDefault).Delete(context.Background(), pvcClaim, metav1.DeleteOptions{}) - Expect(err).ToNot(HaveOccurred()) + f.closeChannel() }) // libguestfs-test-tool verifies the setup to run libguestfs-tools It("Should successfully run libguestfs-test-tool", func() { - f := createFakeAttacher() - defer f.closeChannel() pvcClaim = "pvc-verify" createPVCFilesystem(pvcClaim) - runGuestfsOnPVC(pvcClaim) + runGuestfsOnPVC(f, pvcClaim) output, _, err := execCommandLibguestfsPod(getGuestfsPodName(pvcClaim), []string{"libguestfs-test-tool"}) Expect(err).ToNot(HaveOccurred()) Expect(output).To(ContainSubstring("===== TEST FINISHED OK =====")) - }) It("[posneg:positive][test_id:6480]Should successfully run guestfs command on a filesystem-based PVC", func() { - f := createFakeAttacher() - defer f.closeChannel() pvcClaim = "pvc-fs" createPVCFilesystem(pvcClaim) - runGuestfsOnPVC(pvcClaim) + runGuestfsOnPVC(f, pvcClaim) verifyCanRunOnFSPVC(getGuestfsPodName(pvcClaim)) }) It("[posneg:negative][test_id:6480]Should fail to run the guestfs command on a PVC in use", func() { - f := createFakeAttacher() - defer f.closeChannel() pvcClaim = "pvc-fail-to-run-twice" createPVCFilesystem(pvcClaim) - runGuestfsOnPVC(pvcClaim) + runGuestfsOnPVC(f, pvcClaim) options := []string{"guestfs", pvcClaim, "--namespace", util.NamespaceTestDefault} @@ -176,12 +187,9 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { }) It("[posneg:positive][test_id:6479]Should successfully run guestfs command on a block-based PVC", func() { - f := createFakeAttacher() - defer f.closeChannel() - pvcClaim = "pvc-block" libstorage.CreateBlockPVC(pvcClaim, "500Mi") - runGuestfsOnPVC(pvcClaim) + runGuestfsOnPVC(f, pvcClaim) stdout, stderr, err := execCommandLibguestfsPod(getGuestfsPodName(pvcClaim), []string{"guestfish", "-a", "/dev/vda", "run"}) Expect(stderr).To(Equal("")) Expect(stdout).To(Equal("")) @@ -189,32 +197,31 @@ var _ = SIGDescribe("[rfe_id:6364][[Serial]Guestfs", func() { }) It("Should successfully run guestfs command on a filesystem-based PVC setting the uid", func() { - f := createFakeAttacher() - defer f.closeChannel() pvcClaim = "pvc-fs-with-different-uid" createPVCFilesystem(pvcClaim) - runGuestfsOnPVC(pvcClaim, "--uid", "1002") + runGuestfsOnPVC(f, pvcClaim, "--uid", "1002") verifyCanRunOnFSPVC(getGuestfsPodName(pvcClaim)) }) }) Context("Run libguestfs on PVCs with root", func() { + var f *fakeAttacher BeforeEach(func() { var err error virtClient, err = kubecli.GetKubevirtClient() Expect(err).ToNot(HaveOccurred()) setGroup = false + f = createFakeAttacher() }) AfterEach(func() { - err := virtClient.CoreV1().PersistentVolumeClaims(util.NamespaceTestDefault).Delete(context.Background(), pvcClaim, metav1.DeleteOptions{}) - Expect(err).ToNot(HaveOccurred()) + f.closeChannel() }) It("Should successfully run guestfs command on a filesystem-based PVC with root", func() { f := createFakeAttacher() defer f.closeChannel() pvcClaim = "pvc-fs-with-root" createPVCFilesystem(pvcClaim) - runGuestfsOnPVC(pvcClaim, "--root") + runGuestfsOnPVC(f, pvcClaim, "--root") verifyCanRunOnFSPVC(getGuestfsPodName(pvcClaim)) }) }) -- 2.38.1
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
