diff --git a/api/v2/common_types.go b/api/v2/common_types.go index 2beb286c..459df3b7 100644 --- a/api/v2/common_types.go +++ b/api/v2/common_types.go @@ -6,6 +6,7 @@ package v2 import ( + "math" "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -100,6 +101,34 @@ func ValidateNamespacedResourceMetadata(obj metav1.Object) field.ErrorList { return errorList } +func validateRetainedResourceMonitor( + retainRuntimeResource bool, + monitorPID *int64, + monitorTimestamp metav1.MicroTime, + monitorPath *field.Path, +) field.ErrorList { + errorList := field.ErrorList{} + monitorTimestampSet := !monitorTimestamp.IsZero() + + if monitorPID != nil && (*monitorPID <= 0 || *monitorPID > math.MaxUint32) { + errorList = append(errorList, field.Invalid(monitorPath.Child("monitorPID"), *monitorPID, "monitorPID must be between 1 and 4294967295")) + } + if !retainRuntimeResource && monitorPID != nil { + errorList = append(errorList, field.Forbidden(monitorPath.Child("monitorPID"), "monitorPID can only be set for retained runtime resources")) + } + if !retainRuntimeResource && monitorTimestampSet { + errorList = append(errorList, field.Forbidden(monitorPath.Child("monitorTimestamp"), "monitorTimestamp can only be set for retained runtime resources")) + } + if monitorPID != nil && !monitorTimestampSet { + errorList = append(errorList, field.Required(monitorPath.Child("monitorTimestamp"), "monitorTimestamp must be set when monitorPID is set")) + } + if monitorPID == nil && monitorTimestampSet { + errorList = append(errorList, field.Required(monitorPath.Child("monitorPID"), "monitorPID must be set when monitorTimestamp is set")) + } + + return errorList +} + func validateSameNamespaceResourceReference( reference string, namespace string, diff --git a/api/v2/physical_container_types.go b/api/v2/physical_container_types.go index 477d8e2d..5ca7f033 100644 --- a/api/v2/physical_container_types.go +++ b/api/v2/physical_container_types.go @@ -177,6 +177,14 @@ type PhysicalContainerConfig struct { // RetainRuntimeContainer keeps a runtime container created by this resource in place when the resource is deleted. RetainRuntimeContainer bool `json:"retainRuntimeContainer,omitempty"` + // MonitorPID optionally scopes a retained runtime container to another process lifetime. + // When set, monitorTimestamp must also be set and retainRuntimeContainer must be true. + // The container is stopped but not removed when the monitored process exits. + MonitorPID *int64 `json:"monitorPID,omitempty"` + + // MonitorTimestamp identifies the process in monitorPID and guards against PID reuse. + MonitorTimestamp metav1.MicroTime `json:"monitorTimestamp,omitempty"` + // ImageRef identifies a PhysicalContainerImage in the same namespace using or /. // Cross-namespace references are not supported. ImageRef string `json:"imageRef,omitempty"` @@ -363,6 +371,15 @@ func (pc *PhysicalContainer) Validate(ctx context.Context) field.ErrorList { container := pc.Spec.Container containerPath := specPath.Child("container") + errorList = append( + errorList, + validateRetainedResourceMonitor( + container.RetainRuntimeContainer, + container.MonitorPID, + container.MonitorTimestamp, + containerPath, + )..., + ) errorList = append(errorList, validateSameNamespaceResourceReference(container.ImageRef, pc.Namespace, containerPath.Child("imageRef"))...) if container.ContainerName != "" && !validContainerNameRegexp.MatchString(container.ContainerName) { errorList = append(errorList, field.Invalid(containerPath.Child("containerName"), container.ContainerName, fmt.Sprintf("containerName must match regex '%s'", validContainerName))) diff --git a/api/v2/physical_container_types_test.go b/api/v2/physical_container_types_test.go index d8c2502a..a57c7bd0 100644 --- a/api/v2/physical_container_types_test.go +++ b/api/v2/physical_container_types_test.go @@ -8,6 +8,7 @@ package v2 import ( "context" "testing" + "time" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -16,6 +17,9 @@ import ( ) func TestPhysicalContainerValidate(t *testing.T) { + monitorPID := int64(42) + invalidMonitorPID := int64(0) + monitorTimestamp := metav1.NewMicroTime(time.Now().UTC()) testCases := []struct { name string container PhysicalContainer @@ -31,6 +35,18 @@ func TestPhysicalContainerValidate(t *testing.T) { Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ImageRef: "test-image"}}, }, }, + { + name: "valid retained container monitor", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{Name: "test-container", Namespace: "test-namespace"}, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + RetainRuntimeContainer: true, + MonitorPID: &monitorPID, + MonitorTimestamp: monitorTimestamp, + }}, + }, + }, { name: "valid existing container", container: PhysicalContainer{ @@ -214,6 +230,55 @@ func TestPhysicalContainerValidate(t *testing.T) { }, expectedError: "spec.container.containerName", }, + { + name: "monitor requires retained container", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{Name: "test-container", Namespace: "test-namespace"}, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + MonitorPID: &monitorPID, + MonitorTimestamp: monitorTimestamp, + }}, + }, + expectedError: "spec.container.monitorPID", + }, + { + name: "monitor pid requires timestamp", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{Name: "test-container", Namespace: "test-namespace"}, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + RetainRuntimeContainer: true, + MonitorPID: &monitorPID, + }}, + }, + expectedError: "spec.container.monitorTimestamp", + }, + { + name: "monitor timestamp requires pid", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{Name: "test-container", Namespace: "test-namespace"}, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + RetainRuntimeContainer: true, + MonitorTimestamp: monitorTimestamp, + }}, + }, + expectedError: "spec.container.monitorPID", + }, + { + name: "monitor pid must be valid", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{Name: "test-container", Namespace: "test-namespace"}, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + RetainRuntimeContainer: true, + MonitorPID: &invalidMonitorPID, + MonitorTimestamp: monitorTimestamp, + }}, + }, + expectedError: "spec.container.monitorPID", + }, { name: "invalid container port range size", container: PhysicalContainer{ diff --git a/api/v2/physical_container_volume_types.go b/api/v2/physical_container_volume_types.go index d040b47b..f839a2c4 100644 --- a/api/v2/physical_container_volume_types.go +++ b/api/v2/physical_container_volume_types.go @@ -98,8 +98,9 @@ type PhysicalContainerVolumeConfig struct { // VolumeName is the runtime name to use when creating a new volume. VolumeName string `json:"volumeName,omitempty"` - // RetainRuntimeVolume keeps the created runtime volume in place when this resource is deleted. - RetainRuntimeVolume bool `json:"retainRuntimeVolume,omitempty"` + // RemoveRuntimeVolumeOnDelete removes the created runtime volume when this resource is deleted. + // Created runtime volumes are retained by default. + RemoveRuntimeVolumeOnDelete bool `json:"removeRuntimeVolumeOnDelete,omitempty"` // ReplaceExisting removes an existing runtime volume with volumeName before creating a new one. // Replacement retries non-forced removal while the existing volume is in use and never removes attached containers. diff --git a/api/v2/physical_container_volume_types_test.go b/api/v2/physical_container_volume_types_test.go index f4f8c627..18efda0b 100644 --- a/api/v2/physical_container_volume_types_test.go +++ b/api/v2/physical_container_volume_types_test.go @@ -156,7 +156,7 @@ func TestPhysicalContainerVolumeValidateUpdateRejectsSpecChanges(t *testing.T) { }, } newVolume := oldVolume.DeepCopy() - newVolume.Spec.Volume.RetainRuntimeVolume = true + newVolume.Spec.Volume.RemoveRuntimeVolumeOnDelete = true errorList := newVolume.ValidateUpdate(context.Background(), oldVolume) diff --git a/api/v2/physical_process_types.go b/api/v2/physical_process_types.go index fa71d042..94292f35 100644 --- a/api/v2/physical_process_types.go +++ b/api/v2/physical_process_types.go @@ -97,6 +97,14 @@ type PhysicalProcessConfig struct { // RetainRuntimeProcess keeps a process launched by this resource running when the resource is deleted. RetainRuntimeProcess bool `json:"retainRuntimeProcess,omitempty"` + // MonitorPID optionally scopes a retained runtime process to another process lifetime. + // When set, monitorTimestamp must also be set and retainRuntimeProcess must be true. + // The retained process is stopped when the monitored process exits. + MonitorPID *int64 `json:"monitorPID,omitempty"` + + // MonitorTimestamp identifies the process in monitorPID and guards against PID reuse. + MonitorTimestamp metav1.MicroTime `json:"monitorTimestamp,omitempty"` + // ExecutablePath is the executable path or name to launch. ExecutablePath string `json:"executablePath"` @@ -230,6 +238,15 @@ func (pp *PhysicalProcess) Validate(ctx context.Context) field.ErrorList { processConfig := pp.Spec.Process processPath := specPath.Child("process") + errorList = append( + errorList, + validateRetainedResourceMonitor( + processConfig.RetainRuntimeProcess, + processConfig.MonitorPID, + processConfig.MonitorTimestamp, + processPath, + )..., + ) if strings.TrimSpace(processConfig.ExecutablePath) == "" { errorList = append(errorList, field.Required(processPath.Child("executablePath"), "executablePath must be set")) } diff --git a/api/v2/physical_process_types_test.go b/api/v2/physical_process_types_test.go index ce170c7d..5795371e 100644 --- a/api/v2/physical_process_types_test.go +++ b/api/v2/physical_process_types_test.go @@ -9,6 +9,7 @@ import ( "context" "math" "testing" + "time" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -18,6 +19,8 @@ func TestPhysicalProcessValidate(t *testing.T) { validPID := int64(42) zeroPID := int64(0) largePID := int64(math.MaxUint32) + 1 + monitorPID := int64(43) + monitorTimestamp := metav1.NewMicroTime(time.Now().UTC()) testCases := []struct { name string process PhysicalProcess @@ -43,6 +46,18 @@ func TestPhysicalProcessValidate(t *testing.T) { Spec: PhysicalProcessSpec{PID: &validPID}, }, }, + { + name: "valid retained process monitor", + process: PhysicalProcess{ + ObjectMeta: metav1.ObjectMeta{Name: "test-process", Namespace: "test-namespace"}, + Spec: PhysicalProcessSpec{Process: &PhysicalProcessConfig{ + ExecutablePath: "test-command", + RetainRuntimeProcess: true, + MonitorPID: &monitorPID, + MonitorTimestamp: monitorTimestamp, + }}, + }, + }, { name: "missing namespace", process: PhysicalProcess{ @@ -101,6 +116,55 @@ func TestPhysicalProcessValidate(t *testing.T) { }, expectedError: "spec.process.executablePath", }, + { + name: "monitor requires retained process", + process: PhysicalProcess{ + ObjectMeta: metav1.ObjectMeta{Name: "test-process", Namespace: "test-namespace"}, + Spec: PhysicalProcessSpec{Process: &PhysicalProcessConfig{ + ExecutablePath: "test-command", + MonitorPID: &monitorPID, + MonitorTimestamp: monitorTimestamp, + }}, + }, + expectedError: "spec.process.monitorPID", + }, + { + name: "monitor pid requires timestamp", + process: PhysicalProcess{ + ObjectMeta: metav1.ObjectMeta{Name: "test-process", Namespace: "test-namespace"}, + Spec: PhysicalProcessSpec{Process: &PhysicalProcessConfig{ + ExecutablePath: "test-command", + RetainRuntimeProcess: true, + MonitorPID: &monitorPID, + }}, + }, + expectedError: "spec.process.monitorTimestamp", + }, + { + name: "monitor timestamp requires pid", + process: PhysicalProcess{ + ObjectMeta: metav1.ObjectMeta{Name: "test-process", Namespace: "test-namespace"}, + Spec: PhysicalProcessSpec{Process: &PhysicalProcessConfig{ + ExecutablePath: "test-command", + RetainRuntimeProcess: true, + MonitorTimestamp: monitorTimestamp, + }}, + }, + expectedError: "spec.process.monitorPID", + }, + { + name: "monitor pid must be valid", + process: PhysicalProcess{ + ObjectMeta: metav1.ObjectMeta{Name: "test-process", Namespace: "test-namespace"}, + Spec: PhysicalProcessSpec{Process: &PhysicalProcessConfig{ + ExecutablePath: "test-command", + RetainRuntimeProcess: true, + MonitorPID: &zeroPID, + MonitorTimestamp: monitorTimestamp, + }}, + }, + expectedError: "spec.process.monitorPID", + }, } for _, testCase := range testCases { diff --git a/api/v2/zz_generated.deepcopy.go b/api/v2/zz_generated.deepcopy.go index 72d8e1f4..c36cb4b7 100644 --- a/api/v2/zz_generated.deepcopy.go +++ b/api/v2/zz_generated.deepcopy.go @@ -295,6 +295,12 @@ func (in *PhysicalContainer) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PhysicalContainerConfig) DeepCopyInto(out *PhysicalContainerConfig) { *out = *in + if in.MonitorPID != nil { + in, out := &in.MonitorPID, &out.MonitorPID + *out = new(int64) + **out = **in + } + in.MonitorTimestamp.DeepCopyInto(&out.MonitorTimestamp) if in.Command != nil { in, out := &in.Command, &out.Command *out = make([]string, len(*in)) @@ -946,6 +952,12 @@ func (in *PhysicalProcess) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PhysicalProcessConfig) DeepCopyInto(out *PhysicalProcessConfig) { *out = *in + if in.MonitorPID != nil { + in, out := &in.MonitorPID, &out.MonitorPID + *out = new(int64) + **out = **in + } + in.MonitorTimestamp.DeepCopyInto(&out.MonitorTimestamp) if in.Args != nil { in, out := &in.Args, &out.Args *out = make([]string, len(*in)) diff --git a/controllers/physical_container_controller.go b/controllers/physical_container_controller.go index d2af86c9..aa8d84de 100644 --- a/controllers/physical_container_controller.go +++ b/controllers/physical_container_controller.go @@ -968,18 +968,18 @@ func (r *PhysicalContainerReconciler) createPhysicalContainer( data.progress = physicalContainerOperationCompleted data.failureMessage = "" data.retryAfter = time.Time{} - r.runPhysicalContainerLifecycleMonitor(container, containerID, log) + r.runPhysicalContainerCleanupMonitor(container, containerID, log) } r.queuePhysicalContainerDataResult(container, stateKey, data) } -// Starts a container monitor process that removes the runtime container if this DCP instance terminates unexpectedly. -// Containers the resource does not own past its own lifetime (RetainRuntimeContainer) are left alone. +// Starts a monitor that removes a non-retained runtime container if DCP exits. // Failures are logged but not surfaced, because the monitor is a best-effort reliability enhancement; // the container harvester reclaims orphaned containers in a later session. -func (r *PhysicalContainerReconciler) runPhysicalContainerLifecycleMonitor(container *apiv2.PhysicalContainer, containerID string, log logr.Logger) { - if container.Spec.Container == nil || container.Spec.Container.RetainRuntimeContainer || containerID == "" { +func (r *PhysicalContainerReconciler) runPhysicalContainerCleanupMonitor(container *apiv2.PhysicalContainer, containerID string, log logr.Logger) { + containerConfig := container.Spec.Container + if containerConfig == nil || containerConfig.RetainRuntimeContainer || containerID == "" { return } @@ -987,10 +987,41 @@ func (r *PhysicalContainerReconciler) runPhysicalContainerLifecycleMonitor(conta log.Error(errors.New("process executor is not configured"), "Could not start PhysicalContainer cleanup monitor") return } - dcpproc.RunContainerWatcher(r.processExecutor, containerID, log) } +// Starts a stop-only monitor for a retained runtime container after it has started. +// Failures are logged but not surfaced, because the monitor is a best-effort reliability enhancement. +func (r *PhysicalContainerReconciler) runRetainedPhysicalContainerLifecycleMonitor( + container *apiv2.PhysicalContainer, + containerID string, + log logr.Logger, +) { + containerConfig := container.Spec.Container + if containerConfig == nil || !containerConfig.RetainRuntimeContainer || containerID == "" { + return + } + monitor, found, monitorErr := dcpproc.MonitorTargetFromFields(containerConfig.MonitorPID, containerConfig.MonitorTimestamp) + if monitorErr != nil { + log.Error(monitorErr, "Could not start retained PhysicalContainer lifecycle monitor") + return + } + if !found { + return + } + if r.processExecutor == nil { + log.Error(errors.New("process executor is not configured"), "Could not start retained PhysicalContainer lifecycle monitor") + return + } + dcpproc.RunContainerWatcherForMonitorWithOptions( + r.processExecutor, + monitor, + containerID, + dcpproc.ContainerWatcherOptions{StopOnly: true}, + log, + ) +} + func (r *PhysicalContainerReconciler) removePhysicalContainerForReplacement(ctx context.Context, containerName string, log logr.Logger) error { inspectedContainer, inspectErr := r.inspectPhysicalContainer(ctx, containerName) if errors.Is(inspectErr, containers.ErrNotFound) { @@ -1154,6 +1185,7 @@ func (r *PhysicalContainerReconciler) startPhysicalContainer( data.state = physicalContainerStateStart data.progress = physicalContainerOperationCompleted data.failureMessage = "" + r.runRetainedPhysicalContainerLifecycleMonitor(container, data.containerID, log) } r.queuePhysicalContainerDataResult(container, stateKey, data) diff --git a/controllers/physical_container_volume_controller.go b/controllers/physical_container_volume_controller.go index b14766f6..199deb08 100644 --- a/controllers/physical_container_volume_controller.go +++ b/controllers/physical_container_volume_controller.go @@ -611,7 +611,7 @@ func (r *PhysicalContainerVolumeReconciler) beginPhysicalContainerVolumeRemoval( log logr.Logger, ) objectChange { volumeConfig := volume.Spec.Volume - if volumeConfig == nil || volumeConfig.RetainRuntimeVolume { + if volumeConfig == nil || !volumeConfig.RemoveRuntimeVolumeOnDelete { r.volumeData.DeleteByNamespacedName(volume.NamespacedName()) return deleteFinalizer(volume, physicalContainerVolumeFinalizer, log) } @@ -972,7 +972,7 @@ func physicalContainerVolumeCreationLabels(volume *apiv2.PhysicalContainerVolume volumeConfig := volume.Spec.Volume creationLabels := physicalResourceCreationLabels( volumeConfig.Labels, - volumeConfig.RetainRuntimeVolume, + !volumeConfig.RemoveRuntimeVolumeOnDelete, volume.UID, log, ) diff --git a/controllers/physical_process_controller.go b/controllers/physical_process_controller.go index cbc2ce72..d327a6dd 100644 --- a/controllers/physical_process_controller.go +++ b/controllers/physical_process_controller.go @@ -604,9 +604,7 @@ func (r *PhysicalProcessReconciler) launchPhysicalProcess( data.progress = physicalResourceProgressRunning data.failureMessage = "" data.retryAfter = time.Time{} - if !processConfig.RetainRuntimeProcess { - dcpproc.RunProcessWatcher(r.processExecutor, handle, log) - } + r.runPhysicalProcessLifecycleMonitor(processConfig, handle, log) r.queuePhysicalProcessDataResult(physicalProcess, stateKey, data) if startWaitForExit != nil { startWaitForExit() @@ -614,6 +612,27 @@ func (r *PhysicalProcessReconciler) launchPhysicalProcess( log.V(1).Info("Physical process launched", "PID", handle.Pid, "ExecutablePath", processConfig.ExecutablePath) } +func (r *PhysicalProcessReconciler) runPhysicalProcessLifecycleMonitor( + processConfig *apiv2.PhysicalProcessConfig, + handle process.ProcessHandle, + log logr.Logger, +) { + if !processConfig.RetainRuntimeProcess { + dcpproc.RunProcessWatcher(r.processExecutor, handle, log) + return + } + + monitor, found, monitorErr := dcpproc.MonitorTargetFromFields(processConfig.MonitorPID, processConfig.MonitorTimestamp) + if monitorErr != nil { + log.Error(monitorErr, "Could not start retained PhysicalProcess lifecycle monitor") + return + } + if !found { + return + } + dcpproc.RunProcessWatcherForMonitor(r.processExecutor, monitor, handle, log) +} + func (r *PhysicalProcessReconciler) queuePhysicalProcessDataResult( physicalProcess *apiv2.PhysicalProcess, stateKey physicalProcessDataStateKey, diff --git a/controllers/physical_resource_invalid_state_test.go b/controllers/physical_resource_invalid_state_test.go index 16f18240..91be9f73 100644 --- a/controllers/physical_resource_invalid_state_test.go +++ b/controllers/physical_resource_invalid_state_test.go @@ -165,7 +165,7 @@ func TestInvalidPhysicalContainerVolumeStillHandlesDeletion(t *testing.T) { DeletionTimestamp: &now, }, Spec: apiv2.PhysicalContainerVolumeSpec{ - Volume: &apiv2.PhysicalContainerVolumeConfig{RetainRuntimeVolume: true}, + Volume: &apiv2.PhysicalContainerVolumeConfig{}, }, } data := &physicalContainerVolumeData{ diff --git a/internal/containers/flags/container_runtime.go b/internal/containers/flags/container_runtime.go index 13a5d5e7..1a876d7e 100644 --- a/internal/containers/flags/container_runtime.go +++ b/internal/containers/flags/container_runtime.go @@ -40,6 +40,11 @@ func GetRuntimeFlagValue() RuntimeFlagValue { return runtime } +// SetRuntimeFlagValue sets the container runtime used by subsequent runtime consumers. +func SetRuntimeFlagValue(value RuntimeFlagValue) error { + return runtime.Set(string(value)) +} + func (rf *RuntimeFlagValue) Set(flagValue string) error { if flagValue == string(UnknownRuntime) || slices.ContainsFunc(supportedRuntimeNames, func(name string) bool { return name == strings.ToLower(flagValue) diff --git a/internal/containers/runtimes/runtime.go b/internal/containers/runtimes/runtime.go index fa1a76ea..54ad4f38 100644 --- a/internal/containers/runtimes/runtime.go +++ b/internal/containers/runtimes/runtime.go @@ -79,6 +79,11 @@ func FindAvailableContainerRuntime(ctx context.Context, log logr.Logger, executo return nil, errNoRuntimeFound } + selectedRuntimeErr := flags.SetRuntimeFlagValue(flags.RuntimeFlagValue(availableRuntime.orchestrator.Name())) + if selectedRuntimeErr != nil { + return nil, fmt.Errorf("record selected container runtime: %w", selectedRuntimeErr) + } + log.V(1).Info("Runtime status", "Runtime", availableRuntime.orchestrator.Name(), "Status", availableRuntime.status) return availableRuntime.orchestrator, nil diff --git a/internal/containers/runtimes/runtime_test.go b/internal/containers/runtimes/runtime_test.go new file mode 100644 index 00000000..7fe5b54d --- /dev/null +++ b/internal/containers/runtimes/runtime_test.go @@ -0,0 +1,64 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package runtimes + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + + "github.com/microsoft/dcp/internal/containers" + "github.com/microsoft/dcp/internal/containers/flags" + "github.com/microsoft/dcp/pkg/process" +) + +type testContainerOrchestrator struct { + containers.ContainerOrchestrator + name string + status containers.ContainerRuntimeStatus +} + +func (o *testContainerOrchestrator) IsDefault() bool { + return false +} + +func (o *testContainerOrchestrator) Name() string { + return o.name +} + +func (o *testContainerOrchestrator) CheckStatus(context.Context, containers.CachedRuntimeStatusUsage) containers.ContainerRuntimeStatus { + return o.status +} + +func TestFindAvailableContainerRuntimeRecordsImplicitSelection(t *testing.T) { + originalRuntime := flags.GetRuntimeFlagValue() + originalSupportedRuntimes := supportedRuntimes + t.Cleanup(func() { + supportedRuntimes = originalSupportedRuntimes + require.NoError(t, flags.SetRuntimeFlagValue(originalRuntime)) + }) + + require.NoError(t, flags.SetRuntimeFlagValue(flags.UnknownRuntime)) + supportedRuntimes = map[flags.RuntimeFlagValue]ContainerOrchestratorFactory{ + flags.PodmanRuntime: func(logr.Logger, process.Executor) containers.ContainerOrchestrator { + return &testContainerOrchestrator{ + name: string(flags.PodmanRuntime), + status: containers.ContainerRuntimeStatus{ + Installed: true, + Running: true, + }, + } + }, + } + + orchestrator, findErr := FindAvailableContainerRuntime(context.Background(), logr.Discard(), nil) + + require.NoError(t, findErr) + require.Equal(t, string(flags.PodmanRuntime), orchestrator.Name()) + require.Equal(t, flags.PodmanRuntime, flags.GetRuntimeFlagValue()) +} diff --git a/internal/dcpproc/commands/root.go b/internal/dcpproc/commands/root.go index e9d6f84c..9d4489c2 100644 --- a/internal/dcpproc/commands/root.go +++ b/internal/dcpproc/commands/root.go @@ -23,7 +23,7 @@ func NewRootCmd(log *logger.Logger) (*cobra.Command, error) { rootCmd := &cobra.Command{ SilenceErrors: true, Use: "dcpproc", - Short: "Monitors dcp and cleans up orphaned resources (processes or containers)", + Short: "Monitors dcp and cleans up orphaned resources", Long: `DCP is a developer tool for running multi-service applications. It integrates your code, emulators and containers to give you a development environment diff --git a/internal/dcpproc/dcpproc_api.go b/internal/dcpproc/dcpproc_api.go index d5c8a0e4..06b3ac56 100644 --- a/internal/dcpproc/dcpproc_api.go +++ b/internal/dcpproc/dcpproc_api.go @@ -17,6 +17,7 @@ import ( "github.com/go-logr/logr" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + container_flags "github.com/microsoft/dcp/internal/containers/flags" "github.com/microsoft/dcp/internal/dcppaths" internal_testutil "github.com/microsoft/dcp/internal/testutil" "github.com/microsoft/dcp/pkg/logger" @@ -136,6 +137,7 @@ func RunContainerWatcherForMonitorWithOptions( cmdArgs = append(cmdArgs, "--stop-only") } cmdArgs = append(cmdArgs, getMonitorCmdArgs(monitor)...) + cmdArgs = append(cmdArgs, getContainerRuntimeCmdArgs()...) startErr := startDcpProc(pe, cmdArgs) if startErr != nil { @@ -194,6 +196,14 @@ func getMonitorCmdArgs(monitor process.ProcessHandle) []string { return cmdArgs } +func getContainerRuntimeCmdArgs() []string { + runtime := container_flags.GetRuntimeFlagValue() + if runtime == container_flags.UnknownRuntime { + return nil + } + return []string{container_flags.GetRuntimeFlag(), string(runtime)} +} + func startDcpProc(pe process.Executor, cmdArgs []string) error { dcpPath, dcpPathErr := dcppaths.GetDcpExePath() if dcpPathErr != nil { diff --git a/internal/dcpproc/dcpproc_api_test.go b/internal/dcpproc/dcpproc_api_test.go index 22d98892..e2947200 100644 --- a/internal/dcpproc/dcpproc_api_test.go +++ b/internal/dcpproc/dcpproc_api_test.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "os/exec" + "slices" "strconv" "testing" "time" @@ -17,6 +18,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + container_flags "github.com/microsoft/dcp/internal/containers/flags" "github.com/microsoft/dcp/internal/dcppaths" internal_testutil "github.com/microsoft/dcp/internal/testutil" "github.com/microsoft/dcp/pkg/osutil" @@ -169,6 +171,29 @@ func TestRunContainerWatcherForMonitorWithStopOnly(t *testing.T) { require.Contains(t, dcpProc.Cmd.Args, "--stop-only", "Should include --stop-only flag") } +func TestRunContainerWatcherPassesSelectedRuntime(t *testing.T) { + log := testutil.NewLogForTesting(t.Name()) + ctx, cancel := testutil.GetTestContext(t, 20*time.Second) + defer cancel() + pe := internal_testutil.NewTestProcessExecutor(ctx) + dcppaths.EnableTestPathProbing() + + originalRuntime := container_flags.GetRuntimeFlagValue() + t.Cleanup(func() { + require.NoError(t, container_flags.SetRuntimeFlagValue(originalRuntime)) + }) + require.NoError(t, container_flags.SetRuntimeFlagValue(container_flags.PodmanRuntime)) + + RunContainerWatcher(pe, "test-container-123", log) + + dcpProc, dcpProcErr := findRunningDcp(pe) + require.NoError(t, dcpProcErr) + runtimeFlagIndex := slices.Index(dcpProc.Cmd.Args, container_flags.GetRuntimeFlag()) + require.GreaterOrEqual(t, runtimeFlagIndex, 0) + require.Less(t, runtimeFlagIndex+1, len(dcpProc.Cmd.Args)) + require.Equal(t, string(container_flags.PodmanRuntime), dcpProc.Cmd.Args[runtimeFlagIndex+1]) +} + func TestStopProcessTree(t *testing.T) { log := testutil.NewLogForTesting(t.Name()) ctx, cancel := testutil.GetTestContext(t, 20*time.Second) diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index b3669a12..481f3530 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -4808,6 +4808,19 @@ func schema_microsoft_dcp_api_v2_PhysicalContainerConfig(ref common.ReferenceCal Format: "", }, }, + "monitorPID": { + SchemaProps: spec.SchemaProps{ + Description: "MonitorPID optionally scopes a retained runtime container to another process lifetime. When set, monitorTimestamp must also be set and retainRuntimeContainer must be true. The container is stopped but not removed when the monitored process exits.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "monitorTimestamp": { + SchemaProps: spec.SchemaProps{ + Description: "MonitorTimestamp identifies the process in monitorPID and guards against PID reuse.", + Ref: ref(metav1.MicroTime{}.OpenAPIModelName()), + }, + }, "imageRef": { SchemaProps: spec.SchemaProps{ Description: "ImageRef identifies a PhysicalContainerImage in the same namespace using or /. Cross-namespace references are not supported.", @@ -4973,7 +4986,7 @@ func schema_microsoft_dcp_api_v2_PhysicalContainerConfig(ref common.ReferenceCal }, }, Dependencies: []string{ - v2.ContainerNetworkConnectionConfig{}.OpenAPIModelName(), v2.ContainerPort{}.OpenAPIModelName(), v2.CreateFileSystem{}.OpenAPIModelName(), v2.VolumeMount{}.OpenAPIModelName(), commonapi.EnvVar{}.OpenAPIModelName(), commonapi.Label{}.OpenAPIModelName()}, + v2.ContainerNetworkConnectionConfig{}.OpenAPIModelName(), v2.ContainerPort{}.OpenAPIModelName(), v2.CreateFileSystem{}.OpenAPIModelName(), v2.VolumeMount{}.OpenAPIModelName(), commonapi.EnvVar{}.OpenAPIModelName(), commonapi.Label{}.OpenAPIModelName(), metav1.MicroTime{}.OpenAPIModelName()}, } } @@ -5992,9 +6005,9 @@ func schema_microsoft_dcp_api_v2_PhysicalContainerVolumeConfig(ref common.Refere Format: "", }, }, - "retainRuntimeVolume": { + "removeRuntimeVolumeOnDelete": { SchemaProps: spec.SchemaProps{ - Description: "RetainRuntimeVolume keeps the created runtime volume in place when this resource is deleted.", + Description: "RemoveRuntimeVolumeOnDelete removes the created runtime volume when this resource is deleted. Created runtime volumes are retained by default.", Type: []string{"boolean"}, Format: "", }, @@ -6249,6 +6262,19 @@ func schema_microsoft_dcp_api_v2_PhysicalProcessConfig(ref common.ReferenceCallb Format: "", }, }, + "monitorPID": { + SchemaProps: spec.SchemaProps{ + Description: "MonitorPID optionally scopes a retained runtime process to another process lifetime. When set, monitorTimestamp must also be set and retainRuntimeProcess must be true. The retained process is stopped when the monitored process exits.", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "monitorTimestamp": { + SchemaProps: spec.SchemaProps{ + Description: "MonitorTimestamp identifies the process in monitorPID and guards against PID reuse.", + Ref: ref(metav1.MicroTime{}.OpenAPIModelName()), + }, + }, "executablePath": { SchemaProps: spec.SchemaProps{ Description: "ExecutablePath is the executable path or name to launch.", @@ -6316,7 +6342,7 @@ func schema_microsoft_dcp_api_v2_PhysicalProcessConfig(ref common.ReferenceCallb }, }, Dependencies: []string{ - commonapi.EnvVar{}.OpenAPIModelName()}, + commonapi.EnvVar{}.OpenAPIModelName(), metav1.MicroTime{}.OpenAPIModelName()}, } } diff --git a/plan/v2-resource-plan.md b/plan/v2-resource-plan.md index 9965af6c..6a87b56a 100644 --- a/plan/v2-resource-plan.md +++ b/plan/v2-resource-plan.md @@ -79,29 +79,25 @@ This document tracks the intended direction for DCP V2 resources. The current V2 - `PhysicalContainerImage` provides source image pull and build workflows. The first runtime image ID successfully inspected by the controller is pinned for the resource lifetime and remains the only identity used for later inspection and dependent containers. If that exact image becomes unavailable, the resource reports it unavailable while retaining the published identity and metadata; it never silently pulls or builds a replacement. Delete and recreate the resource to realize a different image. - `PhysicalContainer` creates or tracks one runtime container, reports runtime status and port mappings, and references same-namespace `PhysicalContainerImage`, `PhysicalContainerVolume`, and `PhysicalContainerNetwork` resources. Container creation waits for every referenced physical resource to become ready. Bind mounts continue to use direct host paths, while named volume mounts resolve the referenced volume's observed runtime ID. - `PhysicalContainerNetwork` creates or references one runtime container network and reports its observed identity, driver, and address allocations. Its spec contains exactly one of top-level `networkID` or nested `network` creation config. Networks referenced by runtime ID are always retained. Created networks are retained when `network.retainRuntimeNetwork` is true; otherwise deletion enumerates running and stopped attachments, forcibly disconnects each container without removing it, and then removes the network. Name collisions are terminal unless `network.replaceExisting` is true, in which case the controller safely removes the specifically resolved network before creating its replacement. Runtime adapters classify their own built-in, non-removable networks, and replacement rejects them before disconnecting any attachments. -- `PhysicalContainerVolume` creates or references one runtime container volume and reports its observed identifier, driver, scope, mount point, and creation time. Its spec contains exactly one of top-level `volumeID` or nested `volume` creation config. Volumes referenced by runtime ID are always retained. Created volumes are retained when `volume.retainRuntimeVolume` is true; otherwise deletion retries non-forced removal until the runtime releases the volume. Removal deliberately does not use force because Podman force-removes attached containers. During namespace deletion, each volume retries removal for up to 30 seconds so an externally attached volume cannot block graceful namespace cleanup indefinitely. Name collisions are terminal unless `volume.replaceExisting` is true, in which case the controller safely removes the specifically resolved volume before creating its replacement. Caller-supplied `volume.labels` pass through to created volumes, with reserved persistence, creator-process, and internal resource UID labels set by the controller. +- `PhysicalContainerVolume` creates or references one runtime container volume and reports its observed identifier, driver, scope, mount point, and creation time. Its spec contains exactly one of top-level `volumeID` or nested `volume` creation config. Volumes referenced by runtime ID and newly created volumes are retained by default. Setting `volume.removeRuntimeVolumeOnDelete` opts a created volume into controller-managed deletion. Deletion retries non-forced removal until the runtime releases the volume. Removal deliberately does not use force because Podman force-removes attached containers. During namespace deletion, each removable volume retries removal for up to 30 seconds so an externally attached volume cannot block graceful namespace cleanup indefinitely. Name collisions are terminal unless `volume.replaceExisting` is true, in which case the controller safely removes the specifically resolved volume before creating its replacement. Caller-supplied `volume.labels` pass through to created volumes, with reserved persistence, creator-process, and internal resource UID labels set by the controller. - `PhysicalProcess` launches or references one operating system process and reports its PID, PID-reuse identity timestamp, exit code when available, and lifecycle phase. Its spec contains exactly one of top-level `pid` or nested `process` creation config. Existing processes referenced by PID are observed and always retained when the resource is deleted. Created processes are stopped on deletion and namespace deletion unless `process.retainRuntimeProcess` is true. Deletion never blocks on runtime state: a resource that never took ownership of a running process drops its finalizer without stopping anything. The mutable top-level `stop` request can terminate either mode. Creation supports executable path, arguments, working directory, and environment without importing logical executable or IDE policy. - The physical resources use the shared `Pending`, `Ready`, `Unknown`, and `Failed` phases, specific `Ready` condition reasons, separate in-memory operation progress, and queued work where side effects can block. +- Created physical containers and processes launch best-effort monitor processes when they are configured to remove or stop their runtime object on Kubernetes resource deletion. Retained physical containers and processes can instead specify a monitor PID and identity timestamp; the retained runtime object is stopped, but not removed, when that process exits. Referenced runtime objects and retained resources without an explicit monitor do not launch cleanup monitors. Abandoned networks and volumes remain eligible for workload-scoped cleanup. ## Follow-up roadmap ### Physical resource layer -1. Decide how monitor processes should clean up physical resources after DCP crashes. - - Define how monitor processes are configured and launched for physical resources. - - Decide which physical resources require crash cleanup monitoring. - - Ensure cleanup behavior works when DCP exits unexpectedly and cannot rely on controller finalizers. - -2. Migrate V1 container-network tunnel proxy to V2 physical resources. +1. Migrate V1 container-network tunnel proxy to V2 physical resources. - Keep tunnel-specific behavior in the V1 controller, including dcptun image handling, server proxy process management, TLS, tunnel gRPC calls, status, and endpoint projection. - Delegate common runtime container lifecycle to V2 physical resources instead of creating and managing the proxy container directly through the orchestrator. -3. Migrate V1 container resource lifecycle to V2 physical resources. +2. Migrate V1 container resource lifecycle to V2 physical resources. - Keep V1-specific policy in the V1 controller, including lifecycle keys, persistent and existing container lookup, leases, compatibility status, and V1 API semantics. - Delegate common image/container/network/volume runtime lifecycle to V2 physical resources. - Avoid keeping repeated container creation, start, inspect, watch, stop, and remove logic in multiple V1 controllers. -4. Add logical resource controllers. +3. Add logical resource controllers. - Physical controllers preserve caller-supplied runtime labels and reserve the persistence, creator-process, and internal resource UID labels they need for harvesting and uncertain-create recovery. - Harvesting normally honors the controller-owned persistence label. Network harvesting is the exception: it intentionally ignores that label and removes orphaned networks after their creator exits so persistent networks cannot exhaust the runtime's finite default network allocations. - Build-created images receive persistent, creator-process, and internal UID labels through `build.labels`. Pulling resolves an expected named image and is not a runtime-object creation operation. diff --git a/test/integration/v2_physical_container_controller_test.go b/test/integration/v2_physical_container_controller_test.go index aeff4498..6578b413 100644 --- a/test/integration/v2_physical_container_controller_test.go +++ b/test/integration/v2_physical_container_controller_test.go @@ -9,6 +9,7 @@ import ( "context" "errors" std_slices "slices" + "strconv" "strings" "testing" "time" @@ -26,6 +27,7 @@ import ( internal_testutil "github.com/microsoft/dcp/internal/testutil" ctrl_testutil "github.com/microsoft/dcp/internal/testutil/ctrlutil" "github.com/microsoft/dcp/pkg/commonapi" + "github.com/microsoft/dcp/pkg/osutil" "github.com/microsoft/dcp/pkg/testutil" ) @@ -1234,6 +1236,45 @@ func TestV2PhysicalContainerControllerPreservesCreatedContainerOnDeletion(t *tes require.Empty(t, physicalContainerMonitorProcesses(containerID)) } +func TestV2PhysicalContainerControllerScopesRetainedContainerToMonitorProcess(t *testing.T) { + t.Parallel() + ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) + defer cancel() + + namespace := createActiveV2Namespace(t, ctx, "v2-pctr-retained-monitor") + image := createReadyV2PhysicalContainerImage(t, ctx, namespace.Name, "retained-monitor-image", "retained-monitor-image") + monitorPID := int64(12345) + monitorTimestamp := metav1.NewMicroTime(time.Now().Add(-time.Minute)) + container := &apiv2.PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{Name: "retained-monitor-container", Namespace: namespace.Name}, + Spec: apiv2.PhysicalContainerSpec{Container: &apiv2.PhysicalContainerConfig{ + ImageRef: image.Name, + ContainerName: "v2-pctr-retained-monitor", + RetainRuntimeContainer: true, + MonitorPID: &monitorPID, + MonitorTimestamp: monitorTimestamp, + }}, + } + require.NoError(t, client.Create(ctx, container)) + + updatedContainer := waitPhysicalContainerPhase(t, ctx, container.NamespacedName(), apiv2.PhysicalContainerPhaseRunning) + require.NotNil(t, updatedContainer.Spec.Container.MonitorPID) + require.Equal(t, monitorPID, *updatedContainer.Spec.Container.MonitorPID) + require.True(t, osutil.Within(monitorTimestamp.Time, updatedContainer.Spec.Container.MonitorTimestamp.Time, 2*time.Microsecond)) + containerID := updatedContainer.Status.ContainerID + removeRuntimeContainerOnCleanup(t, containerID) + var monitorProcesses []*internal_testutil.ProcessExecution + waitErr := wait.PollUntilContextCancel(ctx, waitPollInterval, pollImmediately, func(context.Context) (bool, error) { + monitorProcesses = physicalContainerMonitorProcesses(containerID) + return len(monitorProcesses) == 1, nil + }) + require.NoError(t, waitErr) + require.Len(t, monitorProcesses, 1) + require.Contains(t, monitorProcesses[0].Cmd.Args, "--stop-only") + require.Contains(t, monitorProcesses[0].Cmd.Args, strconv.FormatInt(monitorPID, 10)) + require.Contains(t, monitorProcesses[0].Cmd.Args, monitorTimestamp.Time.Format(osutil.RFC3339MiliTimestampFormat)) +} + func physicalContainerMonitorProcesses(containerID string) []*internal_testutil.ProcessExecution { return testProcessExecutor.FindAll([]string{"dcp", "monitor-container"}, "", func(processExecution *internal_testutil.ProcessExecution) bool { return std_slices.Contains(processExecution.Cmd.Args, containerID) diff --git a/test/integration/v2_physical_container_volume_controller_test.go b/test/integration/v2_physical_container_volume_controller_test.go index ff28fac3..7e0fa02f 100644 --- a/test/integration/v2_physical_container_volume_controller_test.go +++ b/test/integration/v2_physical_container_volume_controller_test.go @@ -27,7 +27,7 @@ import ( "github.com/microsoft/dcp/pkg/testutil" ) -func TestV2PhysicalContainerVolumeControllerCreatesVolume(t *testing.T) { +func TestV2PhysicalContainerVolumeControllerCreatesRetainedVolumeByDefault(t *testing.T) { t.Parallel() ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) defer cancel() @@ -64,7 +64,7 @@ func TestV2PhysicalContainerVolumeControllerCreatesVolume(t *testing.T) { inspectedVolume := inspectRuntimeVolume(t, ctx, volumeName) require.Equal(t, "test-value", inspectedVolume.Labels["test-label"]) require.Equal(t, string(readyVolume.UID), inspectedVolume.Labels["com.microsoft.developer.usvc-dev.uid"]) - require.Equal(t, "false", inspectedVolume.Labels[controllers.PersistentLabel]) + require.Equal(t, "true", inspectedVolume.Labels[controllers.PersistentLabel]) require.NotEmpty(t, inspectedVolume.Labels[controllers.CreatorProcessIdLabel]) require.NotEqual(t, "caller-value", inspectedVolume.Labels[controllers.CreatorProcessIdLabel]) require.NotEmpty(t, inspectedVolume.Labels[controllers.CreatorProcessStartTimeLabel]) @@ -96,16 +96,16 @@ func TestV2PhysicalContainerVolumeControllerRetainsReferencedVolume(t *testing.T require.NotNil(t, inspectRuntimeVolume(t, ctx, volumeName)) } -func TestV2PhysicalContainerVolumeControllerDeletesCreatedVolumesUnlessPersistent(t *testing.T) { +func TestV2PhysicalContainerVolumeControllerHonorsCreatedVolumeCleanupPolicy(t *testing.T) { t.Parallel() ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) defer cancel() namespace := createActiveV2Namespace(t, ctx, "v2-pcv-delete") - for _, persistent := range []bool{false, true} { - name := "deleted" - if persistent { - name = "persistent" + for _, removeRuntimeVolumeOnDelete := range []bool{false, true} { + name := "retained" + if removeRuntimeVolumeOnDelete { + name = "removed" } volumeName := "v2-pcv-" + name + "-runtime" removeRuntimeVolumeOnCleanup(t, volumeName) @@ -113,8 +113,8 @@ func TestV2PhysicalContainerVolumeControllerDeletesCreatedVolumesUnlessPersisten ObjectMeta: metav1.ObjectMeta{Name: name + "-volume", Namespace: namespace.Name}, Spec: apiv2.PhysicalContainerVolumeSpec{ Volume: &apiv2.PhysicalContainerVolumeConfig{ - VolumeName: volumeName, - RetainRuntimeVolume: persistent, + VolumeName: volumeName, + RemoveRuntimeVolumeOnDelete: removeRuntimeVolumeOnDelete, }, }, } @@ -123,11 +123,11 @@ func TestV2PhysicalContainerVolumeControllerDeletesCreatedVolumesUnlessPersisten require.NoError(t, client.Delete(ctx, volume)) ctrl_testutil.WaitObjectDeleted[apiv2.PhysicalContainerVolume](t, ctx, client, volume) - if persistent { + if removeRuntimeVolumeOnDelete { + waitRuntimeVolumeMissing(t, ctx, volumeName) + } else { inspectedVolume := inspectRuntimeVolume(t, ctx, volumeName) require.Equal(t, "true", inspectedVolume.Labels[controllers.PersistentLabel]) - } else { - waitRuntimeVolumeMissing(t, ctx, volumeName) } } } @@ -142,7 +142,7 @@ func TestV2PhysicalContainerVolumeControllerWaitsForInUseVolume(t *testing.T) { removeRuntimeVolumeOnCleanup(t, volumeName) volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "in-use-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhaseReady) @@ -190,7 +190,7 @@ func TestV2PhysicalContainerVolumeControllerCleansUpOnNamespaceDeletion(t *testi removeRuntimeVolumeOnCleanup(t, volumeName) volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "namespace-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhaseReady) @@ -214,7 +214,7 @@ func TestV2PhysicalContainerVolumeControllerDoesNotDuplicateCreate(t *testing.T) volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "single-create-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) waitCreateVolumeCallCount(t, ctx, volumeName, 1) @@ -243,7 +243,7 @@ func TestV2PhysicalContainerVolumeControllerWaitsForCreateBeforeDeletion(t *test volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "delete-during-create-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) waitCreateVolumeCallCount(t, ctx, volumeName, 1) @@ -275,7 +275,7 @@ func TestV2PhysicalContainerVolumeControllerAdoptsVolumeAfterUncertainCreateFail volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "uncertain-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) @@ -296,7 +296,7 @@ func TestV2PhysicalContainerVolumeControllerReportsTerminalNameCollision(t *test volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "collision-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) @@ -323,9 +323,8 @@ func TestV2PhysicalContainerVolumeControllerReplacesAndPersistsExistingVolume(t ObjectMeta: metav1.ObjectMeta{Name: "replacement-volume", Namespace: namespace.Name}, Spec: apiv2.PhysicalContainerVolumeSpec{ Volume: &apiv2.PhysicalContainerVolumeConfig{ - VolumeName: volumeName, - RetainRuntimeVolume: true, - ReplaceExisting: true, + VolumeName: volumeName, + ReplaceExisting: true, }, }, } @@ -506,7 +505,7 @@ func TestV2PhysicalContainerVolumeControllerAdoptsSameResourceVolumeAfterStateLo removeRuntimeVolumeOnCleanup(t, volumeName) volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "state-loss-volume", Namespace: namespaceName}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) pendingVolume := waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhasePending) @@ -536,7 +535,7 @@ func TestV2PhysicalContainerVolumeControllerReportsExternalRemovalWithoutRecreat removeRuntimeVolumeOnCleanup(t, volumeName) volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "missing-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) readyVolume := waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhaseReady) @@ -562,7 +561,7 @@ func TestV2PhysicalContainerVolumeControllerDoesNotChurnReadyStatus(t *testing.T removeRuntimeVolumeOnCleanup(t, volumeName) volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "steady-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) readyVolume := waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhaseReady) @@ -603,7 +602,7 @@ func TestV2PhysicalContainerVolumeControllerRecoversFromRuntimeAndCreateFailures volumeName := "v2-pcv-recovery-runtime" volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "recovering-volume", Namespace: namespace.Name}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, serverInfo.Client.Create(ctx, volume)) failedVolume := waitPhysicalContainerVolumeReasonEx(t, ctx, serverInfo.Client, volume.NamespacedName(), apiv2.PhysicalContainerVolumeReasonCreateFailed) @@ -633,7 +632,7 @@ func TestV2PhysicalContainerVolumeControllerWaitsForNamespace(t *testing.T) { removeRuntimeVolumeOnCleanup(t, volumeName) volume := &apiv2.PhysicalContainerVolume{ ObjectMeta: metav1.ObjectMeta{Name: "wait-namespace-volume", Namespace: "v2-pcv-wait-namespace"}, - Spec: newPhysicalContainerVolumeSpec(volumeName), + Spec: newRemovablePhysicalContainerVolumeSpec(volumeName), } require.NoError(t, client.Create(ctx, volume)) waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhasePending) @@ -682,9 +681,12 @@ func waitPhysicalContainerVolumeReasonEx( }) } -func newPhysicalContainerVolumeSpec(volumeName string) apiv2.PhysicalContainerVolumeSpec { +func newRemovablePhysicalContainerVolumeSpec(volumeName string) apiv2.PhysicalContainerVolumeSpec { return apiv2.PhysicalContainerVolumeSpec{ - Volume: &apiv2.PhysicalContainerVolumeConfig{VolumeName: volumeName}, + Volume: &apiv2.PhysicalContainerVolumeConfig{ + VolumeName: volumeName, + RemoveRuntimeVolumeOnDelete: true, + }, } } diff --git a/test/integration/v2_physical_container_volume_durability_test.go b/test/integration/v2_physical_container_volume_durability_test.go index a876774f..ee6a515a 100644 --- a/test/integration/v2_physical_container_volume_durability_test.go +++ b/test/integration/v2_physical_container_volume_durability_test.go @@ -492,7 +492,10 @@ func durablePhysicalContainerVolume(namespace, name, volumeName string) *apiv2.P Finalizers: []string{apiv2.GroupName + "/physicalcontainervolume-reconciler"}, }, Spec: apiv2.PhysicalContainerVolumeSpec{ - Volume: &apiv2.PhysicalContainerVolumeConfig{VolumeName: volumeName}, + Volume: &apiv2.PhysicalContainerVolumeConfig{ + VolumeName: volumeName, + RemoveRuntimeVolumeOnDelete: true, + }, }, } } diff --git a/test/integration/v2_physical_process_controller_test.go b/test/integration/v2_physical_process_controller_test.go index f860e435..10c9f1b3 100644 --- a/test/integration/v2_physical_process_controller_test.go +++ b/test/integration/v2_physical_process_controller_test.go @@ -621,13 +621,17 @@ func TestV2PhysicalProcessControllerDeletesOrRetainsCreatedProcess(t *testing.T) dcppaths.EnableTestPathProbing() dcpPath, dcpPathErr := dcppaths.GetDcpExePath() require.NoError(t, dcpPathErr) + customMonitorPID := int64(12345) testCases := []struct { - name string - retain bool + name string + slug string + retain bool + customMonitorPID *int64 }{ - {name: "deletes", retain: false}, - {name: "retains", retain: true}, + {name: "deletes", slug: "deletes", retain: false}, + {name: "retains", slug: "retains", retain: true}, + {name: "retains with monitor", slug: "retains-with-monitor", retain: true, customMonitorPID: &customMonitorPID}, } for _, testCase := range testCases { @@ -636,19 +640,30 @@ func TestV2PhysicalProcessControllerDeletesOrRetainsCreatedProcess(t *testing.T) ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) defer cancel() - namespace := createActiveV2Namespace(t, ctx, "v2-pproc-"+testCase.name) - executablePath := "v2-pproc-" + testCase.name + "-command" + namespace := createActiveV2Namespace(t, ctx, "v2-pproc-"+testCase.slug) + executablePath := "v2-pproc-" + testCase.slug + "-command" + monitorTimestamp := metav1.NewMicroTime(time.Now().Add(-time.Minute)) physicalProcess := &apiv2.PhysicalProcess{ - ObjectMeta: metav1.ObjectMeta{Name: testCase.name + "-process", Namespace: namespace.Name}, + ObjectMeta: metav1.ObjectMeta{Name: testCase.slug + "-process", Namespace: namespace.Name}, Spec: apiv2.PhysicalProcessSpec{ Process: &apiv2.PhysicalProcessConfig{ ExecutablePath: executablePath, RetainRuntimeProcess: testCase.retain, + MonitorPID: testCase.customMonitorPID, + MonitorTimestamp: monitorTimestamp, }, }, } + if testCase.customMonitorPID == nil { + physicalProcess.Spec.Process.MonitorTimestamp = metav1.MicroTime{} + } require.NoError(t, client.Create(ctx, physicalProcess)) runningProcess := waitPhysicalProcessPhase(t, ctx, physicalProcess.NamespacedName(), apiv2.PhysicalProcessPhaseRunning) + if testCase.customMonitorPID != nil { + require.NotNil(t, runningProcess.Spec.Process.MonitorPID) + require.Equal(t, *testCase.customMonitorPID, *runningProcess.Spec.Process.MonitorPID) + require.True(t, osutil.Within(monitorTimestamp.Time, runningProcess.Spec.Process.MonitorTimestamp.Time, 2*time.Microsecond)) + } pid, convertErr := process.Int64_ToPidT(*runningProcess.Status.PID) require.NoError(t, convertErr) monitorExecutions := testProcessExecutor.FindAll( @@ -656,7 +671,11 @@ func TestV2PhysicalProcessControllerDeletesOrRetainsCreatedProcess(t *testing.T) "", nil, ) - if testCase.retain { + if testCase.customMonitorPID != nil { + require.Len(t, monitorExecutions, 1) + require.Contains(t, monitorExecutions[0].Cmd.Args, strconv.FormatInt(*testCase.customMonitorPID, 10)) + require.Contains(t, monitorExecutions[0].Cmd.Args, monitorTimestamp.Time.Format(osutil.RFC3339MiliTimestampFormat)) + } else if testCase.retain { require.Empty(t, monitorExecutions) } else { require.Len(t, monitorExecutions, 1)