diff --git a/manifests/storage_checkup_permissions.yaml b/manifests/storage_checkup_permissions.yaml index 5acf9d19..c2a76159 100644 --- a/manifests/storage_checkup_permissions.yaml +++ b/manifests/storage_checkup_permissions.yaml @@ -29,7 +29,7 @@ rules: verbs: [ "create", "delete" ] - apiGroups: [ "" ] resources: [ "persistentvolumeclaims" ] - verbs: [ "delete" ] + verbs: [ "create", "get", "delete" ] --- apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding diff --git a/pkg/internal/checkup/checkup.go b/pkg/internal/checkup/checkup.go index 2445dedb..cc0c8e6e 100644 --- a/pkg/internal/checkup/checkup.go +++ b/pkg/internal/checkup/checkup.go @@ -60,6 +60,8 @@ type kubeVirtStorageClient interface { CreateDataVolume(ctx context.Context, namespace string, dv *cdiv1.DataVolume) (*cdiv1.DataVolume, error) DeleteDataVolume(ctx context.Context, namespace, name string) error DeletePersistentVolumeClaim(ctx context.Context, namespace, name string) error + CreatePersistentVolumeClaim(ctx context.Context, namespace string, + pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) ListNodes(ctx context.Context) (*corev1.NodeList, error) ListNamespaces(ctx context.Context) (*corev1.NamespaceList, error) ListStorageClasses(ctx context.Context) (*storagev1.StorageClassList, error) @@ -86,8 +88,10 @@ const ( AnnDefaultVirtStorageClass = "storageclass.kubevirt.io/is-default-virt-class" AnnDefaultStorageClass = "storageclass.kubernetes.io/is-default-class" + AnnSelectedNode = "volume.kubernetes.io/selected-node" ErrNoDefaultStorageClass = "no default storage class" + ErrPVCCapacityMismatch = "PVC capacity is less than requested" ErrPvcNotBound = "pvc failed to bound" ErrMultipleDefaultStorageClasses = "there are multiple default storage classes" ErrEmptyClaimPropertySets = "there are StorageProfiles with empty ClaimPropertySets (unknown provisioners)" @@ -165,6 +169,11 @@ func (c *Checkup) Run(ctx context.Context) error { return err } + err = c.checkPVCCapacity(ctx, &errStr) + if err != nil { + return err + } + sps, err := c.client.ListStorageProfiles(ctx) if err != nil { return err @@ -883,6 +892,117 @@ func (c *Checkup) checkVMIBoot(ctx context.Context, errStr *string) error { return nil } +func (c *Checkup) checkPVCCapacity(ctx context.Context, errStr *string) error { + log.Print("checkPVCCapacity") + + if c.defaultStorageClass == "" && c.checkupConfig.StorageClass == "" { + log.Print(MessageSkipNoDefaultStorageClass) + c.results.PVCCapacity = MessageSkipNoDefaultStorageClass + return nil + } + + requestedSize := "12345Mi" + + scName := c.defaultStorageClass + if c.checkupConfig.StorageClass != "" { + scName = c.checkupConfig.StorageClass + } + + nodeName, err := c.getSchedulableNode(ctx) + if err != nil { + return err + } + + pvcName := fmt.Sprintf("capacity-check-pvc-%s", rand.String(5)) //nolint:mnd + blockMode := corev1.PersistentVolumeBlock + pvc := &corev1.PersistentVolumeClaim{ + ObjectMeta: metav1.ObjectMeta{ + Name: pvcName, + Annotations: map[string]string{ + AnnSelectedNode: nodeName, + }, + }, + Spec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce}, + StorageClassName: &scName, + VolumeMode: &blockMode, + Resources: corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse(requestedSize), + }, + }, + }, + } + + log.Printf("checkPVCCapacity: creating PVC %q (%s) with SC %q on node %q", pvcName, requestedSize, scName, nodeName) + if _, err = c.client.CreatePersistentVolumeClaim(ctx, c.namespace, pvc); err != nil { + return fmt.Errorf("failed to create PVC %q: %w", pvcName, err) + } + defer func() { + if delErr := c.client.DeletePersistentVolumeClaim(ctx, c.namespace, pvcName); delErr != nil { + log.Printf("checkPVCCapacity: failed to delete PVC %q: %v", pvcName, delErr) + } + }() + + if err = wait.PollImmediateWithContext(ctx, pollInterval, c.checkupConfig.VMITimeout, func(ctx context.Context) (bool, error) { + p, getErr := c.client.GetPersistentVolumeClaim(ctx, c.namespace, pvcName) + if getErr != nil { + return false, getErr + } + return p.Status.Phase == corev1.ClaimBound, nil + }); err != nil { + res := fmt.Sprintf("PVC %q failed to bind: %v", pvcName, err) + log.Print(res) + appendSep(&c.results.PVCCapacity, res) + appendSep(errStr, ErrPvcNotBound) + return nil + } + + boundPVC, err := c.client.GetPersistentVolumeClaim(ctx, c.namespace, pvcName) + if err != nil { + return fmt.Errorf("failed to get PVC %q: %w", pvcName, err) + } + + capacity := boundPVC.Status.Capacity[corev1.ResourceStorage] + capacityMi := capacity.Value() / (1024 * 1024) + + if capacity.Cmp(resource.MustParse(requestedSize)) < 0 { + res := fmt.Sprintf("PVC %q: requested %s, got %dMi - capacity mismatch", pvcName, requestedSize, capacityMi) + log.Print(res) + appendSep(&c.results.PVCCapacity, res) + appendSep(errStr, ErrPVCCapacityMismatch) + } else { + res := fmt.Sprintf("PVC %q: requested %s, got %dMi", pvcName, requestedSize, capacityMi) + log.Print(res) + appendSep(&c.results.PVCCapacity, res) + } + + return nil +} + +func (c *Checkup) getSchedulableNode(ctx context.Context) (string, error) { + nodes, err := c.client.ListNodes(ctx) + if err != nil { + return "", fmt.Errorf("failed to list nodes: %w", err) + } + for i := range nodes.Items { + node := &nodes.Items[i] + if !node.Spec.Unschedulable && isNodeReady(node) { + return node.Name, nil + } + } + return "", errors.New("no ready schedulable nodes found") +} + +func isNodeReady(node *corev1.Node) bool { + for i := range node.Status.Conditions { + if node.Status.Conditions[i].Type == corev1.NodeReady { + return node.Status.Conditions[i].Status == corev1.ConditionTrue + } + } + return false +} + func (c *Checkup) checkVMILiveMigration(ctx context.Context, errStr *string) error { log.Print("checkVMILiveMigration") diff --git a/pkg/internal/checkup/checkup_test.go b/pkg/internal/checkup/checkup_test.go index 22d6c7fc..8fc33eb1 100644 --- a/pkg/internal/checkup/checkup_test.go +++ b/pkg/internal/checkup/checkup_test.go @@ -32,6 +32,7 @@ import ( corev1 "k8s.io/api/core/v1" storagev1 "k8s.io/api/storage/v1" "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -78,8 +79,12 @@ func TestCheckupShouldSucceed(t *testing.T) { assert.Empty(t, testClient.createdVMs) assert.Empty(t, testClient.createdVMIs) - expectedResults := successfulRunResults(vmiUnderTestName) actualResults := reporter.FormatResults(testCheckup.Results()) + assert.Contains(t, actualResults[reporter.PVCCapacityKey], "requested 12345Mi, got 12345Mi") + delete(actualResults, reporter.PVCCapacityKey) + + expectedResults := successfulRunResults(vmiUnderTestName) + delete(expectedResults, reporter.PVCCapacityKey) assert.Equal(t, expectedResults, actualResults) } @@ -87,12 +92,14 @@ var tests = map[string]struct { clientConfig clientConfig expectedResults map[string]string expectedErr string + resultsContains bool }{ "noStorageClasses": { clientConfig: clientConfig{noStorageClasses: true, expectNoVMI: true}, expectedResults: map[string]string{ reporter.DefaultStorageClassKey: checkup.ErrNoDefaultStorageClass, reporter.PVCBoundKey: checkup.MessageSkipNoDefaultStorageClass, + reporter.PVCCapacityKey: checkup.MessageSkipNoDefaultStorageClass, reporter.VMBootFromGoldenImageKey: checkup.MessageSkipNoDefaultStorageClass, reporter.ConcurrentVMBootKey: checkup.MessageSkipNoDefaultStorageClass, }, @@ -103,6 +110,7 @@ var tests = map[string]struct { expectedResults: map[string]string{ reporter.DefaultStorageClassKey: checkup.ErrNoDefaultStorageClass, reporter.PVCBoundKey: checkup.MessageSkipNoDefaultStorageClass, + reporter.PVCCapacityKey: checkup.MessageSkipNoDefaultStorageClass, reporter.VMBootFromGoldenImageKey: checkup.MessageSkipNoDefaultStorageClass, reporter.ConcurrentVMBootKey: checkup.MessageSkipNoDefaultStorageClass, }, @@ -186,6 +194,14 @@ var tests = map[string]struct { expectedResults: map[string]string{reporter.VMLiveMigrationKey: "Skip check - single node"}, expectedErr: "", }, + "pvcCapacityMismatch": { + clientConfig: clientConfig{pvcCapacityMismatch: true}, + resultsContains: true, + expectedResults: map[string]string{ + reporter.PVCCapacityKey: "requested 12345Mi, got 512Mi - capacity mismatch", + }, + expectedErr: checkup.ErrPVCCapacityMismatch, + }, } func TestCheckupShouldReturnErrorWhen(t *testing.T) { @@ -207,10 +223,20 @@ func TestCheckupShouldReturnErrorWhen(t *testing.T) { checkOwnerRef(t, testClient) } - expectedResults := fullExpectedResults(vmiUnderTestName, tc.expectedResults) actualResults := reporter.FormatResults(testCheckup.Results()) - - assert.Equal(t, expectedResults, actualResults) + if tc.resultsContains { + for key, substr := range tc.expectedResults { + substr = strings.ReplaceAll(substr, "%s", vmiUnderTestName) + assert.Contains(t, actualResults[key], substr, "key %s", key) + } + } else { + expectedResults := fullExpectedResults(vmiUnderTestName, tc.expectedResults) + assert.Contains(t, actualResults[reporter.PVCCapacityKey], + expectedResults[reporter.PVCCapacityKey]) + delete(actualResults, reporter.PVCCapacityKey) + delete(expectedResults, reporter.PVCCapacityKey) + assert.Equal(t, expectedResults, actualResults) + } if tc.expectedErr != "" { assert.ErrorContains(t, err, tc.expectedErr) } else { @@ -273,6 +299,7 @@ func successfulRunResults(vmiUnderTestName string) map[string]string { reporter.VMHotplugVolumeKey: fmt.Sprintf("VMI %q hotplug volume ready\nVMI %q hotplug volume removed", vmiUnderTestName, vmiUnderTestName), reporter.ConcurrentVMBootKey: "Boot completed on all VMs on time", + reporter.PVCCapacityKey: "requested 12345Mi, got 12345Mi", } } @@ -295,6 +322,7 @@ type clientConfig struct { cloneFallback bool failMigration bool singleNode bool + pvcCapacityMismatch bool } type clientStub struct { @@ -455,6 +483,13 @@ func (cs *clientStub) DeletePersistentVolumeClaim(ctx context.Context, namespace return nil } +func (cs *clientStub) CreatePersistentVolumeClaim(ctx context.Context, namespace string, + pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) { + pvc.Namespace = namespace + pvc.Status.Phase = corev1.ClaimBound + return pvc, nil +} + func (cs *clientStub) ListNodes(ctx context.Context) (*corev1.NodeList, error) { nodeList := &corev1.NodeList{} itemCount := 2 @@ -466,6 +501,12 @@ func (cs *clientStub) ListNodes(ctx context.Context) (*corev1.NodeList, error) { ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("node-%d", i), }, + Status: corev1.NodeStatus{ + Conditions: []corev1.NodeCondition{{ + Type: corev1.NodeReady, + Status: corev1.ConditionTrue, + }}, + }, }) } @@ -668,10 +709,25 @@ func (cs *clientStub) GetPersistentVolumeClaim(ctx context.Context, namespace, n }, Status: corev1.PersistentVolumeClaimStatus{ AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteMany}, + Capacity: corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("1Gi"), + }, }, } - if cs.failPvcBound { + if strings.HasPrefix(name, "capacity-check-pvc-") { + if cs.pvcCapacityMismatch { + pvc.Status.Capacity = corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("512Mi"), + } + } else { + pvc.Status.Capacity = corev1.ResourceList{ + corev1.ResourceStorage: resource.MustParse("12345Mi"), + } + } + } + + if cs.failPvcBound && !strings.HasPrefix(name, "capacity-check-pvc-") { pvc.Status.Phase = corev1.ClaimPending } else { pvc.Status.Phase = corev1.ClaimBound diff --git a/pkg/internal/client/client.go b/pkg/internal/client/client.go index 7e8d9c20..58c010cc 100644 --- a/pkg/internal/client/client.go +++ b/pkg/internal/client/client.go @@ -100,6 +100,11 @@ func (c *Client) DeletePersistentVolumeClaim(ctx context.Context, namespace, nam return c.CoreV1().PersistentVolumeClaims(namespace).Delete(ctx, name, metav1.DeleteOptions{}) } +func (c *Client) CreatePersistentVolumeClaim(ctx context.Context, namespace string, + pvc *corev1.PersistentVolumeClaim) (*corev1.PersistentVolumeClaim, error) { + return c.CoreV1().PersistentVolumeClaims(namespace).Create(ctx, pvc, metav1.CreateOptions{}) +} + func (c *Client) ListNodes(ctx context.Context) (*corev1.NodeList, error) { return c.CoreV1().Nodes().List(ctx, metav1.ListOptions{}) } diff --git a/pkg/internal/reporter/reporter.go b/pkg/internal/reporter/reporter.go index 68a62c2e..13db5df5 100644 --- a/pkg/internal/reporter/reporter.go +++ b/pkg/internal/reporter/reporter.go @@ -46,6 +46,7 @@ const ( VMLiveMigrationKey = "vmLiveMigration" VMHotplugVolumeKey = "vmHotplugVolume" ConcurrentVMBootKey = "concurrentVMBoot" + PVCCapacityKey = "pvcCapacity" ) type Reporter struct { @@ -95,6 +96,7 @@ func FormatResults(checkupResults status.Results) map[string]string { VMLiveMigrationKey: checkupResults.VMLiveMigration, VMHotplugVolumeKey: checkupResults.VMHotplugVolume, ConcurrentVMBootKey: checkupResults.ConcurrentVMBoot, + PVCCapacityKey: checkupResults.PVCCapacity, } return formattedResults diff --git a/pkg/internal/reporter/reporter_test.go b/pkg/internal/reporter/reporter_test.go index dd9968f4..b1328018 100644 --- a/pkg/internal/reporter/reporter_test.go +++ b/pkg/internal/reporter/reporter_test.go @@ -79,6 +79,7 @@ func TestReportShouldSuccessfullyReportResults(t *testing.T) { VMLiveMigration: "success", VMHotplugVolume: "fail", ConcurrentVMBoot: "ok", + PVCCapacity: "ok", } assert.NoError(t, testReporter.Report(checkupStatus)) @@ -105,6 +106,7 @@ func TestReportShouldSuccessfullyReportResults(t *testing.T) { "status.result.vmLiveMigration": checkupStatus.Results.VMLiveMigration, "status.result.vmHotplugVolume": checkupStatus.Results.VMHotplugVolume, "status.result.concurrentVMBoot": checkupStatus.Results.ConcurrentVMBoot, + "status.result.pvcCapacity": checkupStatus.Results.PVCCapacity, } assert.Equal(t, expectedReportData, getCheckupData(t, fakeClient, testNamespace, testConfigMapName)) }) diff --git a/pkg/internal/status/status.go b/pkg/internal/status/status.go index c0f6b985..a9cb2317 100644 --- a/pkg/internal/status/status.go +++ b/pkg/internal/status/status.go @@ -42,6 +42,7 @@ type Results struct { VMLiveMigration string VMHotplugVolume string ConcurrentVMBoot string + PVCCapacity string } type Status struct { diff --git a/tests/checkup_test.go b/tests/checkup_test.go index 96155434..d40a8f87 100644 --- a/tests/checkup_test.go +++ b/tests/checkup_test.go @@ -206,7 +206,7 @@ func newCheckupRole() *rbacv1.Role { { APIGroups: []string{""}, Resources: []string{"persistentvolumeclaims"}, - Verbs: []string{"delete"}, + Verbs: []string{"create", "get", "delete"}, }, }, }