Skip to content

update GeneratePVCName method #72

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.

Sign up for GitHub

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

Open
ZhaoQing7892 wants to merge 4 commits into
base: master
Choose a base branch
from
+293 −106
Open

update GeneratePVCName method #72

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
088aee0
update GeneratePVCName method
ZhaoQing7892 Jan 7, 2026
0bcfff7
add e2e for pvctemplate
ZhaoQing7892 Jan 8, 2026
bef1a70
add e2e for pvctemplate
ZhaoQing7892 Jan 9, 2026
352bb43
add e2e for pvctemplate
ZhaoQing7892 Jan 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions pkg/controller/sandbox/core/common_control.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,7 @@ func (r *commonControl) createPod(ctx context.Context, box *agentsv1alpha1.Sandb

volumes := make([]corev1.Volume, 0, len(box.Spec.VolumeClaimTemplates))
for _, template := range box.Spec.VolumeClaimTemplates {
pvcName, err := GeneratePVCName(template.Name, box.Name)
if err != nil {
logger.Error(err, "failed to generate PVC name", "template", template.Name, "sandbox", box.Name)
return nil, err
}
pvcName := GeneratePVCName(template.Name, box.Name)
volumes = append(volumes, corev1.Volume{
Name: template.Name,
VolumeSource: corev1.VolumeSource{
Expand Down
5 changes: 1 addition & 4 deletions pkg/controller/sandbox/core/common_control_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -830,10 +830,7 @@ func TestCommonControl_createPod(t *testing.T) {
t.Fatalf("createPod() with PVC error = %v", err)
}

expectedPVCName, err := GeneratePVCName("www", "test-sandbox-with-pvc")
if err != nil {
t.Fatalf("GeneratePVCName() error = %v", err)
}
expectedPVCName := GeneratePVCName("www", "test-sandbox-with-pvc")

if len(podWithPVC.Spec.Volumes) != 1 {
t.Errorf("Expected 1 volume, got %d", len(podWithPVC.Spec.Volumes))
Expand Down
10 changes: 2 additions & 8 deletions pkg/controller/sandbox/core/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,6 @@ func HashSandbox(box *agentsv1alpha1.Sandbox) (string, string) {
}

// GeneratePVCName generates a persistent volume claim name from template name and sandbox name
func GeneratePVCName(templateName, sandboxName string) (string, error) {
if templateName == "" || sandboxName == "" {
return "", fmt.Errorf("template name and sandbox name cannot be empty")
}

name := fmt.Sprintf("%s-%s", templateName, sandboxName)

return name, nil
func GeneratePVCName(templateName, sandboxName string) string {
return fmt.Sprintf("%s-%s", templateName, sandboxName)
}
73 changes: 0 additions & 73 deletions pkg/controller/sandbox/core/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,76 +340,3 @@ func TestHashSandboxWithDifferentResources(t *testing.T) {
hashWithoutImageResources1, hashWithoutImageResources2)
}
}

func TestGeneratePVCName(t *testing.T) {
tests := []struct {
name string
templateName string
sandboxName string
expectError bool
expectName string
}{
{
name: "normal case",
templateName: "www",
sandboxName: "test-sandbox",
expectError: false,
expectName: "www-test-sandbox",
},
{
name: "template name with hyphen",
templateName: "data-volume",
sandboxName: "test-sandbox",
expectError: false,
expectName: "data-volume-test-sandbox",
},
{
name: "sandbox name with number",
templateName: "cache",
sandboxName: "app-123",
expectError: false,
expectName: "cache-app-123",
},
{
name: "empty template name",
templateName: "",
sandboxName: "test-sandbox",
expectError: true,
},
{
name: "empty sandbox name",
templateName: "www",
sandboxName: "",
expectError: true,
},
{
name: "both empty names",
templateName: "",
sandboxName: "",
expectError: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
result, err := GeneratePVCName(tt.templateName, tt.sandboxName)

if tt.expectError {
if err == nil {
t.Errorf("Expected error but got none")
}
// Verify that the error message is meaningful
if err != nil && err.Error() == "" {
t.Errorf("Expected error message but got empty string")
}
} else {
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if result != tt.expectName {
t.Errorf("Expected name %s, but got %s", tt.expectName, result)
}
}
})
}
}
19 changes: 3 additions & 16 deletions pkg/controller/sandbox/sandbox_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,19 +332,12 @@ func (r *SandboxReconciler) SetupWithManager(mgr ctrl.Manager) error {
// ensureVolumeClaimTemplates creates and ensures PVCs exist for persistent data recovery during sleep/wake operations
func (r *SandboxReconciler) ensureVolumeClaimTemplates(ctx context.Context, box *agentsv1alpha1.Sandbox) error {
logger := logf.FromContext(ctx).WithValues("sandbox", klog.KObj(box))

if len(box.Spec.VolumeClaimTemplates) == 0 {
return nil
}

for _, template := range box.Spec.VolumeClaimTemplates {
// Generate PVC name based on template name and sandbox name
pvcName, err := core.GeneratePVCName(template.Name, box.Name)
if err != nil {
logger.Error(err, "failed to generate PVC name", "template", template.Name, "sandbox", box.Name)
return err
}

pvcName := core.GeneratePVCName(template.Name, box.Name)
// Create PVC object based on the template
pvc := &corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -353,32 +346,26 @@ func (r *SandboxReconciler) ensureVolumeClaimTemplates(ctx context.Context, box
},
Spec: template.Spec,
}

// Set the sandbox as the owner of the PVC to align their lifecycles
if err = ctrl.SetControllerReference(box, pvc, r.Scheme); err != nil {
if err := ctrl.SetControllerReference(box, pvc, r.Scheme); err != nil {
logger.Error(err, "failed to set sandbox as owner of PVC", "pvc", pvcName)
return err
}

// Check if PVC already exists
existingPVC := &corev1.PersistentVolumeClaim{}
err = r.Get(ctx, client.ObjectKey{Namespace: box.Namespace, Name: pvcName}, existingPVC)

err := r.Get(ctx, client.ObjectKey{Namespace: box.Namespace, Name: pvcName}, existingPVC)
if err == nil {
logger.Info("PVC already exists for persistent data recovery", "pvc", pvcName)
continue
}

if !errors.IsNotFound(err) {
logger.Error(err, "failed to get PVC", "pvc", pvcName)
return err
}

if err = r.Create(ctx, pvc); err == nil {
logger.Info("created PVC for persistent data recovery", "pvc", pvcName)
continue
}

if !errors.IsAlreadyExists(err) {
logger.Error(err, "failed to create PVC", "pvc", pvcName)
return err
Expand Down
147 changes: 147 additions & 0 deletions test/e2e/sandbox.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
agentsv1alpha1 "github.com/openkruise/agents/api/v1alpha1"
"github.com/openkruise/agents/pkg/utils"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/rand"
Expand Down Expand Up @@ -348,7 +349,7 @@
}
initialPodName = pod.Name
for _, container := range pod.Spec.Containers {
if container.Name == "test-container" && container.Image == initialImage {

Check failure on line 352 in test/e2e/sandbox.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

string `test-container` has 3 occurrences, make it a constant (goconst)
return container.Image
}
}
Expand Down Expand Up @@ -431,6 +432,152 @@
})
})

Context("with volume claim templates", func() {
It("should create sandbox with PVCs successfully", func() {
By("Creating a Sandbox with VolumeClaimTemplates")
sandboxWithPVC := &agentsv1alpha1.Sandbox{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("test-sandbox-pvc-%d", time.Now().UnixNano()),
Namespace: namespace,
},
Spec: agentsv1alpha1.SandboxSpec{
SandboxTemplate: agentsv1alpha1.SandboxTemplate{
Template: &corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
{
Name: "test-container",
Image: "nginx:stable-alpine3.23",
VolumeMounts: []corev1.VolumeMount{
{
Name: "www",
MountPath: "/usr/share/nginx/html",
},
},
},
},
// 添加 Volumes 定义
Volumes: []corev1.Volume{
{
Name: "www",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "www",
},
},
},
},
},
},
VolumeClaimTemplates: []corev1.PersistentVolumeClaim{
{
ObjectMeta: metav1.ObjectMeta{
Name: "www",
},
Spec: corev1.PersistentVolumeClaimSpec{
AccessModes: []corev1.PersistentVolumeAccessMode{corev1.ReadWriteOnce},
Resources: corev1.VolumeResourceRequirements{
Requests: corev1.ResourceList{
corev1.ResourceStorage: resource.MustParse("1Gi"),
},
},
},
},
},
},
},
}
Expect(k8sClient.Create(ctx, sandboxWithPVC)).To(Succeed())

By("Waiting for sandbox to reach Running phase")
Eventually(func() agentsv1alpha1.SandboxPhase {
_ = k8sClient.Get(ctx, types.NamespacedName{
Name: sandboxWithPVC.Name,
Namespace: sandboxWithPVC.Namespace,
}, sandboxWithPVC)
return sandboxWithPVC.Status.Phase
}, time.Second*90, time.Millisecond*500).Should(Equal(agentsv1alpha1.SandboxRunning))

By("Verifying PVC is created and bound")
pvcName := "www-" + sandboxWithPVC.Name
Eventually(func() corev1.PersistentVolumeClaimPhase {
pvc := &corev1.PersistentVolumeClaim{}
err := k8sClient.Get(ctx, types.NamespacedName{
Name: pvcName,
Namespace: sandboxWithPVC.Namespace,
}, pvc)
if err != nil {
return ""
}
return pvc.Status.Phase
}, time.Second*60, time.Millisecond*500).Should(Equal(corev1.ClaimBound))

By("Verifying pod is running with PVC mounted")
pod := &corev1.Pod{}
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: sandboxWithPVC.Name,
Namespace: sandboxWithPVC.Namespace,
}, pod)).To(Succeed())

Expect(pod.Spec.Volumes).ToNot(BeEmpty())
foundVolume := false
for _, volume := range pod.Spec.Volumes {
if volume.Name == "www" && volume.PersistentVolumeClaim != nil {

Check failure on line 525 in test/e2e/sandbox.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

string `www` has 4 occurrences, make it a constant (goconst)
foundVolume = true
Expect(volume.PersistentVolumeClaim.ClaimName).To(Equal(pvcName))
break
}
}
Expect(foundVolume).To(BeTrue())

Expect(pod.Spec.Containers).ToNot(BeEmpty())
container := pod.Spec.Containers[0]
foundMount := false
for _, mount := range container.VolumeMounts {
if mount.Name == "www" && mount.MountPath == "/usr/share/nginx/html" {
foundMount = true
break
}
}
Expect(foundMount).To(BeTrue())

By("Pausing and resuming sandbox to verify data persistence")
// Pause the Sandbox
originalSandbox := &agentsv1alpha1.Sandbox{}
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: sandboxWithPVC.Name,
Namespace: sandboxWithPVC.Namespace,
}, originalSandbox)).To(Succeed())

originalSandbox.Spec.Paused = true
Expect(updateSandboxSpec(ctx, originalSandbox)).To(Succeed())

Eventually(func() agentsv1alpha1.SandboxPhase {
_ = k8sClient.Get(ctx, types.NamespacedName{
Name: sandboxWithPVC.Name,
Namespace: sandboxWithPVC.Namespace,
}, sandboxWithPVC)
return sandboxWithPVC.Status.Phase
}, time.Second*30, time.Millisecond*500).Should(Equal(agentsv1alpha1.SandboxPaused))

// Resume the Sandbox
Expect(k8sClient.Get(ctx, types.NamespacedName{
Name: sandboxWithPVC.Name,
Namespace: sandboxWithPVC.Namespace,
}, originalSandbox)).To(Succeed())

originalSandbox.Spec.Paused = false
Expect(updateSandboxSpec(ctx, originalSandbox)).To(Succeed())

Eventually(func() agentsv1alpha1.SandboxPhase {
_ = k8sClient.Get(ctx, types.NamespacedName{
Name: sandboxWithPVC.Name,
Namespace: sandboxWithPVC.Namespace,
}, sandboxWithPVC)
return sandboxWithPVC.Status.Phase
}, time.Second*60, time.Millisecond*500).Should(Equal(agentsv1alpha1.SandboxRunning))
})
})
})

func createNamespace(ctx context.Context) string {
Expand Down
Loading
Loading