diff --git a/api/v2/common_types.go b/api/v2/common_types.go index 2266abab..2beb286c 100644 --- a/api/v2/common_types.go +++ b/api/v2/common_types.go @@ -12,6 +12,8 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" + + "github.com/microsoft/dcp/pkg/commonapi" ) // ConditionType identifies a condition reported by a V2 resource. @@ -97,3 +99,28 @@ func ValidateNamespacedResourceMetadata(obj metav1.Object) field.ErrorList { return errorList } + +func validateSameNamespaceResourceReference( + reference string, + namespace string, + referencePath *field.Path, +) field.ErrorList { + if reference == "" { + return field.ErrorList{field.Required(referencePath, "reference must be set")} + } + + namespacedName := commonapi.AsNamespacedName(reference, namespace) + errorList := field.ErrorList{} + if strings.Contains(reference, string(types.Separator)) { + for _, validationMessage := range validation.IsDNS1123Label(namespacedName.Namespace) { + errorList = append(errorList, field.Invalid(referencePath, reference, validationMessage)) + } + if namespacedName.Namespace != namespace { + errorList = append(errorList, field.Invalid(referencePath, reference, "cross-namespace references are not supported")) + } + } + for _, validationMessage := range validation.IsDNS1123Subdomain(namespacedName.Name) { + errorList = append(errorList, field.Invalid(referencePath, reference, validationMessage)) + } + return errorList +} diff --git a/api/v2/common_types_test.go b/api/v2/common_types_test.go index c90e762d..c2bde73a 100644 --- a/api/v2/common_types_test.go +++ b/api/v2/common_types_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/validation/field" "github.com/microsoft/dcp/pkg/commonapi" ) @@ -68,6 +69,73 @@ func TestValidateNamespacedResourceMetadata(t *testing.T) { } } +func TestValidateSameNamespaceResourceReference(t *testing.T) { + testCases := []struct { + name string + reference string + expectedError string + }{ + { + name: "name", + reference: "test-resource", + }, + { + name: "explicit same namespace", + reference: "test-namespace/test-resource", + }, + { + name: "missing reference", + expectedError: "reference must be set", + }, + { + name: "cross namespace", + reference: "other-namespace/test-resource", + expectedError: "cross-namespace references are not supported", + }, + { + name: "missing explicit namespace", + reference: "/test-resource", + expectedError: "lowercase RFC 1123 label", + }, + { + name: "missing explicit name", + reference: "test-namespace/", + expectedError: "lowercase RFC 1123 subdomain", + }, + { + name: "additional separator", + reference: "test-namespace/test-resource/extra", + expectedError: "lowercase RFC 1123 subdomain", + }, + { + name: "invalid explicit namespace", + reference: "INVALID/test-resource", + expectedError: "lowercase RFC 1123 label", + }, + { + name: "invalid name", + reference: "INVALID", + expectedError: "lowercase RFC 1123 subdomain", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + errorList := validateSameNamespaceResourceReference( + tc.reference, + "test-namespace", + field.NewPath("spec", "reference"), + ) + if tc.expectedError == "" { + require.Empty(t, errorList) + } else { + require.NotEmpty(t, errorList) + require.Contains(t, errorList.ToAggregate().Error(), tc.expectedError) + } + }) + } +} + func TestResourceCreationProhibited(t *testing.T) { commonapi.ResourceCreationProhibited.Store(true) defer commonapi.ResourceCreationProhibited.Store(false) diff --git a/api/v2/container_types.go b/api/v2/container_types.go index f4992b45..e4f4f36e 100644 --- a/api/v2/container_types.go +++ b/api/v2/container_types.go @@ -36,9 +36,12 @@ const ( type VolumeMount struct { Type VolumeMountType `json:"type"` - // Bind mounts: the host directory to mount. - // Volume mounts: name of the volume to mount. - Source string `json:"source"` + // Source is the host directory to mount for bind mounts. + Source string `json:"source,omitempty"` + + // VolumeRef identifies a PhysicalContainerVolume in the same namespace using or /. + // Cross-namespace references are not supported. + VolumeRef string `json:"volumeRef,omitempty"` // The path within the container that the mount will use. Target string `json:"target"` @@ -48,6 +51,32 @@ type VolumeMount struct { ReadOnly bool `json:"readOnly,omitempty"` } +func ValidateVolumeMounts(mounts []VolumeMount, namespace string, mountsPath *field.Path) field.ErrorList { + errorList := field.ErrorList{} + + for i, mount := range mounts { + mountPath := mountsPath.Index(i) + switch mount.Type { + case BindMount: + if mount.Source == "" { + errorList = append(errorList, field.Required(mountPath.Child("source"), "source must be set for bind mounts")) + } + if mount.VolumeRef != "" { + errorList = append(errorList, field.Forbidden(mountPath.Child("volumeRef"), "volumeRef cannot be set for bind mounts")) + } + case NamedVolumeMount: + if mount.Source != "" { + errorList = append(errorList, field.Forbidden(mountPath.Child("source"), "source cannot be set for volume mounts")) + } + errorList = append(errorList, validateSameNamespaceResourceReference(mount.VolumeRef, namespace, mountPath.Child("volumeRef"))...) + default: + errorList = append(errorList, field.NotSupported(mountPath.Child("type"), mount.Type, []string{string(BindMount), string(NamedVolumeMount)})) + } + } + + return errorList +} + // ContainerPort describes a port, or contiguous range of ports, to publish from a container. // +k8s:openapi-gen=true type ContainerPort struct { @@ -130,7 +159,8 @@ func ValidateContainerPorts(ports []ContainerPort, portsPath *field.Path) field. // ContainerNetworkConnectionConfig describes a PhysicalContainerNetwork to attach to a container. // +k8s:openapi-gen=true type ContainerNetworkConnectionConfig struct { - // Name of the PhysicalContainerNetwork to connect to in the container's namespace. + // Name identifies a PhysicalContainerNetwork in the same namespace using or /. + // Cross-namespace references are not supported. Name string `json:"name"` // Aliases of the container on the network. diff --git a/api/v2/doc.go b/api/v2/doc.go index 1b362bac..bf3e1e4c 100644 --- a/api/v2/doc.go +++ b/api/v2/doc.go @@ -7,10 +7,8 @@ // // The V2 API is unstable and under active development. Its resources, fields, and // semantics may change in breaking ways between DCP releases until the physical and -// logical resource layers described in docs/v2-resource-plan.md are complete. In -// particular, PhysicalContainer is expected to replace direct runtime network and -// volume names with references to V2 resources. Callers outside DCP should not depend -// on V2 API stability yet. +// logical resource layers described in plan/v2-resource-plan.md are complete. Callers +// outside DCP should not depend on V2 API stability yet. // // +kubebuilder:object:generate=true // +groupName=usvc-dev.developer.microsoft.com diff --git a/api/v2/physical_container_network_connection_types.go b/api/v2/physical_container_network_connection_types.go index 38a4514a..21a1cbe7 100644 --- a/api/v2/physical_container_network_connection_types.go +++ b/api/v2/physical_container_network_connection_types.go @@ -13,7 +13,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" apiserver_resource "github.com/tilt-dev/tilt-apiserver/pkg/server/builder/resource" @@ -27,10 +26,12 @@ import ( // Both references resolve within the connection resource's namespace. // +k8s:openapi-gen=true type PhysicalContainerNetworkConnectionSpec struct { - // ContainerRef is the name of the PhysicalContainer to connect. + // ContainerRef identifies the PhysicalContainer using or /. + // Cross-namespace references are not supported. ContainerRef string `json:"containerRef"` - // NetworkRef is the name of the PhysicalContainerNetwork to connect to. + // NetworkRef identifies the PhysicalContainerNetwork using or /. + // Cross-namespace references are not supported. NetworkRef string `json:"networkRef"` // Aliases contains network-scoped aliases for the container. @@ -101,8 +102,8 @@ func (connection *PhysicalContainerNetworkConnection) Validate(ctx context.Conte } errorList = append(errorList, commonapi.ValidateAnnotationsSize(connection.Annotations, field.NewPath("metadata", "annotations"))...) - errorList = append(errorList, validatePhysicalResourceReference(connection.Spec.ContainerRef, specPath.Child("containerRef"))...) - errorList = append(errorList, validatePhysicalResourceReference(connection.Spec.NetworkRef, specPath.Child("networkRef"))...) + errorList = append(errorList, validateSameNamespaceResourceReference(connection.Spec.ContainerRef, connection.Namespace, specPath.Child("containerRef"))...) + errorList = append(errorList, validateSameNamespaceResourceReference(connection.Spec.NetworkRef, connection.Namespace, specPath.Child("networkRef"))...) return errorList } @@ -117,18 +118,6 @@ func (connection *PhysicalContainerNetworkConnection) ValidateUpdate(ctx context } } -func validatePhysicalResourceReference(reference string, referencePath *field.Path) field.ErrorList { - if reference == "" { - return field.ErrorList{field.Required(referencePath, "reference must be set")} - } - - errorList := field.ErrorList{} - for _, validationMessage := range validation.IsDNS1123Subdomain(reference) { - errorList = append(errorList, field.Invalid(referencePath, reference, validationMessage)) - } - return errorList -} - // PhysicalContainerNetworkConnectionList contains a list of PhysicalContainerNetworkConnection instances. // +k8s:openapi-gen=true // +kubebuilder:object:root=true diff --git a/api/v2/physical_container_network_connection_types_test.go b/api/v2/physical_container_network_connection_types_test.go index e965b477..ca2962a1 100644 --- a/api/v2/physical_container_network_connection_types_test.go +++ b/api/v2/physical_container_network_connection_types_test.go @@ -33,6 +33,17 @@ func TestPhysicalContainerNetworkConnectionValidate(t *testing.T) { }, valid: true, }, + { + name: "valid explicit same namespace references", + connection: PhysicalContainerNetworkConnection{ + ObjectMeta: metav1.ObjectMeta{Name: "connection", Namespace: "namespace"}, + Spec: PhysicalContainerNetworkConnectionSpec{ + ContainerRef: "namespace/container", + NetworkRef: "namespace/network", + }, + }, + valid: true, + }, { name: "missing container reference", connection: PhysicalContainerNetworkConnection{ @@ -71,6 +82,26 @@ func TestPhysicalContainerNetworkConnectionValidate(t *testing.T) { }, }, }, + { + name: "cross-namespace container reference", + connection: PhysicalContainerNetworkConnection{ + ObjectMeta: metav1.ObjectMeta{Name: "connection", Namespace: "namespace"}, + Spec: PhysicalContainerNetworkConnectionSpec{ + ContainerRef: "other-namespace/container", + NetworkRef: "network", + }, + }, + }, + { + name: "cross-namespace network reference", + connection: PhysicalContainerNetworkConnection{ + ObjectMeta: metav1.ObjectMeta{Name: "connection", Namespace: "namespace"}, + Spec: PhysicalContainerNetworkConnectionSpec{ + ContainerRef: "container", + NetworkRef: "other-namespace/network", + }, + }, + }, } for _, test := range tests { diff --git a/api/v2/physical_container_types.go b/api/v2/physical_container_types.go index 5c96aad0..477d8e2d 100644 --- a/api/v2/physical_container_types.go +++ b/api/v2/physical_container_types.go @@ -16,7 +16,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/validation" "k8s.io/apimachinery/pkg/util/validation/field" apiserver_resource "github.com/tilt-dev/tilt-apiserver/pkg/server/builder/resource" @@ -65,6 +64,15 @@ const ( // PhysicalContainerReasonImageLookupFailed indicates that the referenced PhysicalContainerImage could not be read. PhysicalContainerReasonImageLookupFailed ConditionReason = "ImageLookupFailed" + // PhysicalContainerReasonVolumeNotFound indicates that a referenced PhysicalContainerVolume does not exist. + PhysicalContainerReasonVolumeNotFound ConditionReason = "VolumeNotFound" + + // PhysicalContainerReasonVolumeNotReady indicates that a referenced PhysicalContainerVolume is not ready. + PhysicalContainerReasonVolumeNotReady ConditionReason = "VolumeNotReady" + + // PhysicalContainerReasonVolumeLookupFailed indicates that a referenced PhysicalContainerVolume could not be read. + PhysicalContainerReasonVolumeLookupFailed ConditionReason = "VolumeLookupFailed" + // PhysicalContainerReasonNetworkNotFound indicates that a referenced PhysicalContainerNetwork does not exist. PhysicalContainerReasonNetworkNotFound ConditionReason = "NetworkNotFound" @@ -169,7 +177,8 @@ type PhysicalContainerConfig struct { // RetainRuntimeContainer keeps a runtime container created by this resource in place when the resource is deleted. RetainRuntimeContainer bool `json:"retainRuntimeContainer,omitempty"` - // ImageRef is the name of a PhysicalContainerImage in the same namespace to use when creating a new runtime container. + // ImageRef identifies a PhysicalContainerImage in the same namespace using or /. + // Cross-namespace references are not supported. ImageRef string `json:"imageRef,omitempty"` // ContainerName is the runtime name to use when creating a new container. @@ -354,13 +363,7 @@ func (pc *PhysicalContainer) Validate(ctx context.Context) field.ErrorList { container := pc.Spec.Container containerPath := specPath.Child("container") - if container.ImageRef == "" { - errorList = append(errorList, field.Required(containerPath.Child("imageRef"), "imageRef must be set")) - } else { - for _, validationMessage := range validation.IsDNS1123Subdomain(container.ImageRef) { - errorList = append(errorList, field.Invalid(containerPath.Child("imageRef"), container.ImageRef, validationMessage)) - } - } + 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))) } @@ -370,8 +373,9 @@ func (pc *PhysicalContainer) Validate(ctx context.Context) field.ErrorList { networksPath := containerPath.Child("networks") for i, network := range container.Networks { - errorList = append(errorList, validatePhysicalResourceReference(network.Name, networksPath.Index(i).Child("name"))...) + errorList = append(errorList, validateSameNamespaceResourceReference(network.Name, pc.Namespace, networksPath.Index(i).Child("name"))...) } + errorList = append(errorList, ValidateVolumeMounts(container.VolumeMounts, pc.Namespace, containerPath.Child("volumeMounts"))...) errorList = append(errorList, ValidateContainerPorts(container.Ports, containerPath.Child("ports"))...) errorList = append(errorList, validateLabels(container.Labels, containerPath.Child("labels"))...) diff --git a/api/v2/physical_container_types_test.go b/api/v2/physical_container_types_test.go index 13e276db..d8c2502a 100644 --- a/api/v2/physical_container_types_test.go +++ b/api/v2/physical_container_types_test.go @@ -62,6 +62,40 @@ func TestPhysicalContainerValidate(t *testing.T) { }, }, }, + { + name: "valid created container with bind and volume mounts", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{ + {Type: BindMount, Source: "/host/data", Target: "/data"}, + {Type: NamedVolumeMount, VolumeRef: "test-volume", Target: "/cache"}, + }, + }}, + }, + }, + { + name: "valid explicit same namespace references", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-namespace/test-image", + VolumeMounts: []VolumeMount{ + {Type: NamedVolumeMount, VolumeRef: "test-namespace/test-volume", Target: "/cache"}, + }, + Networks: []ContainerNetworkConnectionConfig{ + {Name: "test-namespace/test-network"}, + }, + }}, + }, + }, { name: "missing namespace", container: PhysicalContainer{ @@ -104,6 +138,17 @@ func TestPhysicalContainerValidate(t *testing.T) { }, expectedError: "spec.container.imageRef", }, + { + name: "cross-namespace imageRef", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ImageRef: "other-namespace/test-image"}}, + }, + expectedError: "cross-namespace references are not supported", + }, { name: "imageRef conflicts with existing container ID", container: PhysicalContainer{ @@ -237,6 +282,128 @@ func TestPhysicalContainerValidate(t *testing.T) { }, expectedError: "spec.container.containerName", }, + { + name: "bind mount requires source", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{{Type: BindMount, Target: "/data"}}, + }}, + }, + expectedError: "spec.container.volumeMounts[0].source", + }, + { + name: "bind mount rejects volumeRef", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{{ + Type: BindMount, Source: "/host/data", VolumeRef: "test-volume", Target: "/data", + }}, + }}, + }, + expectedError: "spec.container.volumeMounts[0].volumeRef", + }, + { + name: "volume mount requires volumeRef", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{{Type: NamedVolumeMount, Target: "/data"}}, + }}, + }, + expectedError: "spec.container.volumeMounts[0].volumeRef", + }, + { + name: "volume mount rejects source", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{{ + Type: NamedVolumeMount, Source: "runtime-volume", VolumeRef: "test-volume", Target: "/data", + }}, + }}, + }, + expectedError: "spec.container.volumeMounts[0].source", + }, + { + name: "volume mount rejects invalid volumeRef", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{{ + Type: NamedVolumeMount, VolumeRef: "/", Target: "/data", + }}, + }}, + }, + expectedError: "spec.container.volumeMounts[0].volumeRef", + }, + { + name: "volume mount rejects cross-namespace volumeRef", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{{ + Type: NamedVolumeMount, VolumeRef: "other-namespace/test-volume", Target: "/data", + }}, + }}, + }, + expectedError: "cross-namespace references are not supported", + }, + { + name: "network rejects cross-namespace reference", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + Networks: []ContainerNetworkConnectionConfig{{ + Name: "other-namespace/test-network", + }}, + }}, + }, + expectedError: "cross-namespace references are not supported", + }, + { + name: "unsupported volume mount type", + container: PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-container", + Namespace: "test-namespace", + }, + Spec: PhysicalContainerSpec{Container: &PhysicalContainerConfig{ + ImageRef: "test-image", + VolumeMounts: []VolumeMount{{Type: VolumeMountType("unsupported"), Target: "/data"}}, + }}, + }, + expectedError: "spec.container.volumeMounts[0].type", + }, { name: "missing label key", container: PhysicalContainer{ diff --git a/controllers/physical_container_controller.go b/controllers/physical_container_controller.go index 7570392e..d2af86c9 100644 --- a/controllers/physical_container_controller.go +++ b/controllers/physical_container_controller.go @@ -40,6 +40,7 @@ import ( const ( physicalContainerImageRefField = ".spec.imageRef" + physicalContainerVolumeRefField = ".spec.volumeMounts.volumeRef" physicalContainerNetworkRefField = ".spec.networks.name" ) @@ -49,8 +50,9 @@ var ( physicalContainerDataInitializers = map[physicalContainerState]physicalContainerDataInitializerFunc{ physicalContainerStateNamespace: handlePhysicalContainerNamespace, physicalContainerStateResolve: handlePhysicalContainerResolve, - physicalContainerStateImage: handlePhysicalContainerImage, - physicalContainerStateNetworks: handlePhysicalContainerNetworks, + physicalContainerStateImage: handlePhysicalContainerPrepare, + physicalContainerStateVolumes: handlePhysicalContainerPrepare, + physicalContainerStateNetworks: handlePhysicalContainerPrepare, physicalContainerStateCreate: handlePhysicalContainerCreate, physicalContainerStateReplace: handlePhysicalContainerCreateFailure, physicalContainerStateCopyFiles: handlePhysicalContainerCopyFiles, @@ -110,7 +112,8 @@ func (r *PhysicalContainerReconciler) SetupWithManager(mgr ctrl.Manager, name st return nil } - return []string{container.Spec.Container.ImageRef} + imageName := commonapi.AsNamespacedName(container.Spec.Container.ImageRef, container.Namespace) + return []string{imageName.Name} }); err != nil { r.Log.Error(err, "Failed to create imageRef index for PhysicalContainer", "IndexField", physicalContainerImageRefField) return err @@ -125,7 +128,8 @@ func (r *PhysicalContainerReconciler) SetupWithManager(mgr ctrl.Manager, name st networkNames := make([]string, 0, len(container.Spec.Container.Networks)) for i := range container.Spec.Container.Networks { if container.Spec.Container.Networks[i].Name != "" { - networkNames = append(networkNames, container.Spec.Container.Networks[i].Name) + networkName := commonapi.AsNamespacedName(container.Spec.Container.Networks[i].Name, container.Namespace) + networkNames = append(networkNames, networkName.Name) } } return networkNames @@ -134,6 +138,26 @@ func (r *PhysicalContainerReconciler) SetupWithManager(mgr ctrl.Manager, name st return err } + if err := indexer.IndexField(context.Background(), &apiv2.PhysicalContainer{}, physicalContainerVolumeRefField, func(rawObj ctrl_client.Object) []string { + container := rawObj.(*apiv2.PhysicalContainer) + if container.Spec.Container == nil || len(container.Spec.Container.VolumeMounts) == 0 { + return nil + } + + volumeNames := make([]string, 0, len(container.Spec.Container.VolumeMounts)) + for i := range container.Spec.Container.VolumeMounts { + mount := &container.Spec.Container.VolumeMounts[i] + if mount.Type == apiv2.NamedVolumeMount && mount.VolumeRef != "" { + volumeName := commonapi.AsNamespacedName(mount.VolumeRef, container.Namespace) + volumeNames = append(volumeNames, volumeName.Name) + } + } + return volumeNames + }); err != nil { + r.Log.Error(err, "Failed to create volumeRef index for PhysicalContainer", "IndexField", physicalContainerVolumeRefField) + return err + } + if err := indexer.IndexField(context.Background(), &apiv2.PhysicalContainerNetworkConnection{}, ownerKey, func(rawObj ctrl_client.Object) []string { connection := rawObj.(*apiv2.PhysicalContainerNetworkConnection) owner := metav1.GetControllerOf(connection) @@ -151,6 +175,7 @@ func (r *PhysicalContainerReconciler) SetupWithManager(mgr ctrl.Manager, name st For(&apiv2.PhysicalContainer{}). Owns(&apiv2.PhysicalContainerNetworkConnection{}). Watches(&apiv2.PhysicalContainerImage{}, handler.EnqueueRequestsFromMapFunc(r.requestReconcileForImage), builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). + Watches(&apiv2.PhysicalContainerVolume{}, handler.EnqueueRequestsFromMapFunc(r.requestReconcileForVolume), builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). Watches(&apiv2.PhysicalContainerNetwork{}, handler.EnqueueRequestsFromMapFunc(r.requestReconcileForNetwork), builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). Watches(&apiv2.Namespace{}, handler.EnqueueRequestsFromMapFunc(r.mapNamespaceToReconcileRequests(&apiv2.PhysicalContainerList{})), builder.WithPredicates(predicate.ResourceVersionChangedPredicate{})). WatchesRawSource(r.GetReconciliationEventSource()). @@ -158,6 +183,24 @@ func (r *PhysicalContainerReconciler) SetupWithManager(mgr ctrl.Manager, name st Complete(r) } +func (r *PhysicalContainerReconciler) requestReconcileForVolume(ctx context.Context, obj ctrl_client.Object) []reconcile.Request { + volume := obj.(*apiv2.PhysicalContainerVolume) + var containerList apiv2.PhysicalContainerList + listErr := r.List(ctx, &containerList, ctrl_client.InNamespace(volume.Namespace), ctrl_client.MatchingFields{physicalContainerVolumeRefField: volume.Name}) + if listErr != nil { + r.Log.Error(listErr, "Failed to list PhysicalContainers referencing PhysicalContainerVolume", "Volume", volume.NamespacedName()) + return nil + } + + requests := make([]reconcile.Request, 0, len(containerList.Items)) + for i := range containerList.Items { + requests = append(requests, reconcile.Request{NamespacedName: containerList.Items[i].NamespacedName()}) + } + + r.Log.V(1).Info("PhysicalContainerVolume updated, requesting PhysicalContainer reconciliation", "Volume", volume.NamespacedName(), "Containers", len(requests)) + return requests +} + func (r *PhysicalContainerReconciler) requestReconcileForNetwork(ctx context.Context, obj ctrl_client.Object) []reconcile.Request { network := obj.(*apiv2.PhysicalContainerNetwork) var containerList apiv2.PhysicalContainerList @@ -335,7 +378,7 @@ func handlePhysicalContainerResolve( containerID = data.containerID } if containerID == "" { - return handlePhysicalContainerImage(ctx, reconciler, container, data.state, data, log) + return handlePhysicalContainerPrepare(ctx, reconciler, container, data.state, data, log) } if data.containerID == "" { @@ -363,7 +406,7 @@ func handlePhysicalContainerResolve( return handlePhysicalContainerRuntime(ctx, reconciler, container, data.state, data, log) } -func handlePhysicalContainerImage( +func handlePhysicalContainerPrepare( ctx context.Context, reconciler *PhysicalContainerReconciler, container *apiv2.PhysicalContainer, @@ -371,6 +414,10 @@ func handlePhysicalContainerImage( data *physicalContainerData, log logr.Logger, ) objectChange { + data.image = "" + data.volumeMounts = nil + data.networks = nil + imageReady, image, imageProgress, imageMessage, imageChange := reconciler.resolvePhysicalContainerImage(ctx, container, log) if !imageReady { data.state = physicalContainerStateImage @@ -379,31 +426,26 @@ func handlePhysicalContainerImage( return imageChange } - data.image = image - data.state = physicalContainerStateNetworks - data.progress = physicalResourceProgressInProgress - data.failureMessage = "" - return imageChange | handlePhysicalContainerNetworks(ctx, reconciler, container, data.state, data, log) -} + volumesReady, volumeMounts, volumeProgress, volumeMessage := reconciler.resolvePhysicalContainerVolumes(ctx, container, log) + if !volumesReady { + data.state = physicalContainerStateVolumes + data.progress = volumeProgress + data.failureMessage = volumeMessage + return imageChange + } -func handlePhysicalContainerNetworks( - ctx context.Context, - reconciler *PhysicalContainerReconciler, - container *apiv2.PhysicalContainer, - _ physicalContainerState, - data *physicalContainerData, - log logr.Logger, -) objectChange { networksReady, networks, networkProgress, networkMessage := reconciler.resolvePhysicalContainerNetworks(ctx, container, log) if !networksReady { data.state = physicalContainerStateNetworks data.progress = networkProgress data.failureMessage = networkMessage - return noChange + return imageChange } + data.image = image + data.volumeMounts = volumeMounts data.networks = networks - return reconciler.schedulePhysicalContainerCreate(container, data, log) + return imageChange | reconciler.schedulePhysicalContainerCreate(container, data, log) } func handlePhysicalContainerCreate( @@ -668,7 +710,7 @@ func handlePhysicalContainerRecoverableCreateFailed( } log.V(1).Info("Retrying physical container creation", "ContainerName", container.Spec.Container.ContainerName) - return cleanupChange | reconciler.schedulePhysicalContainerCreate(container, data, log) + return cleanupChange | handlePhysicalContainerPrepare(ctx, reconciler, container, data.state, data, log) } func (r *PhysicalContainerReconciler) removePartiallyCreatedPhysicalContainer( @@ -738,7 +780,8 @@ func (r *PhysicalContainerReconciler) resolvePhysicalContainerImage( ) (bool, string, physicalResourceProgress, string, objectChange) { image := apiv2.PhysicalContainerImage{} imageRef := container.Spec.Container.ImageRef - getErr := r.Client.Get(ctx, types.NamespacedName{Namespace: container.Namespace, Name: imageRef}, &image) + imageName := commonapi.AsNamespacedName(imageRef, container.Namespace) + getErr := r.Client.Get(ctx, imageName, &image) if apierrors.IsNotFound(getErr) { return false, "", physicalResourceProgressNotFound, fmt.Sprintf("PhysicalContainerImage %q does not exist.", imageRef), noChange } @@ -746,6 +789,9 @@ func (r *PhysicalContainerReconciler) resolvePhysicalContainerImage( log.Error(getErr, "Failed to get PhysicalContainerImage", "ImageRef", imageRef) return false, "", physicalResourceProgressRetryPending, fmt.Sprintf("Failed to get PhysicalContainerImage: %v", getErr), additionalReconciliationNeeded } + if image.DeletionTimestamp != nil && !image.DeletionTimestamp.IsZero() { + return false, "", physicalResourceProgressNotReady, fmt.Sprintf("PhysicalContainerImage %q is terminating.", imageRef), noChange + } if image.Status.Phase != apiv2.PhysicalContainerImagePhaseReady || image.Status.ImageID == "" { return false, "", physicalResourceProgressNotReady, fmt.Sprintf("PhysicalContainerImage %q is not ready.", imageRef), noChange } @@ -753,6 +799,47 @@ func (r *PhysicalContainerReconciler) resolvePhysicalContainerImage( return true, image.Status.ImageID, physicalResourceProgressCompleted, "", setValue(&container.Status.Image, image.Status.ImageID) } +func (r *PhysicalContainerReconciler) resolvePhysicalContainerVolumes( + ctx context.Context, + container *apiv2.PhysicalContainer, + log logr.Logger, +) (bool, []containers.CreateContainerVolumeMount, physicalResourceProgress, string) { + containerConfig := container.Spec.Container + volumeMounts := make([]containers.CreateContainerVolumeMount, 0, len(containerConfig.VolumeMounts)) + for i := range containerConfig.VolumeMounts { + mount := &containerConfig.VolumeMounts[i] + source := mount.Source + if mount.Type == apiv2.NamedVolumeMount { + volume := apiv2.PhysicalContainerVolume{} + volumeName := commonapi.AsNamespacedName(mount.VolumeRef, container.Namespace) + getErr := r.Client.Get(ctx, volumeName, &volume) + if apierrors.IsNotFound(getErr) { + return false, nil, physicalResourceProgressNotFound, fmt.Sprintf("PhysicalContainerVolume %q does not exist.", mount.VolumeRef) + } + if getErr != nil { + log.Error(getErr, "Failed to get PhysicalContainerVolume", "VolumeRef", mount.VolumeRef) + return false, nil, physicalResourceProgressRetryPending, fmt.Sprintf("Failed to get PhysicalContainerVolume %q: %v", mount.VolumeRef, getErr) + } + if volume.DeletionTimestamp != nil && !volume.DeletionTimestamp.IsZero() { + return false, nil, physicalResourceProgressNotReady, fmt.Sprintf("PhysicalContainerVolume %q is terminating.", mount.VolumeRef) + } + if volume.Status.Phase != apiv2.PhysicalContainerVolumePhaseReady || volume.Status.VolumeID == "" { + return false, nil, physicalResourceProgressNotReady, fmt.Sprintf("PhysicalContainerVolume %q is not ready.", mount.VolumeRef) + } + source = volume.Status.VolumeID + } + + volumeMounts = append(volumeMounts, containers.CreateContainerVolumeMount{ + Type: containers.VolumeMountType(mount.Type), + Source: source, + Target: mount.Target, + ReadOnly: mount.ReadOnly, + }) + } + + return true, volumeMounts, physicalResourceProgressCompleted, "" +} + func (r *PhysicalContainerReconciler) resolvePhysicalContainerNetworks( ctx context.Context, container *apiv2.PhysicalContainer, @@ -763,7 +850,7 @@ func (r *PhysicalContainerReconciler) resolvePhysicalContainerNetworks( for i := range containerConfig.Networks { networkConfig := &containerConfig.Networks[i] network := apiv2.PhysicalContainerNetwork{} - networkName := types.NamespacedName{Namespace: container.Namespace, Name: networkConfig.Name} + networkName := commonapi.AsNamespacedName(networkConfig.Name, container.Namespace) getErr := r.Client.Get(ctx, networkName, &network) if apierrors.IsNotFound(getErr) { return false, nil, physicalResourceProgressNotFound, fmt.Sprintf("PhysicalContainerNetwork %q does not exist.", networkConfig.Name) @@ -772,6 +859,9 @@ func (r *PhysicalContainerReconciler) resolvePhysicalContainerNetworks( log.Error(getErr, "Failed to get PhysicalContainerNetwork", "NetworkRef", networkConfig.Name) return false, nil, physicalResourceProgressRetryPending, fmt.Sprintf("Failed to get PhysicalContainerNetwork %q: %v", networkConfig.Name, getErr) } + if network.DeletionTimestamp != nil && !network.DeletionTimestamp.IsZero() { + return false, nil, physicalResourceProgressNotReady, fmt.Sprintf("PhysicalContainerNetwork %q is terminating.", networkConfig.Name) + } if network.Status.Phase != apiv2.PhysicalContainerNetworkPhaseReady || network.Status.NetworkID == "" { return false, nil, physicalResourceProgressNotReady, fmt.Sprintf("PhysicalContainerNetwork %q is not ready.", networkConfig.Name) } @@ -845,7 +935,7 @@ func (r *PhysicalContainerReconciler) createPhysicalContainer( Image: data.image, Entrypoint: containerConfig.Entrypoint, Command: containerConfig.Command, - VolumeMounts: physicalVolumeMountsToCreateContainerVolumeMounts(containerConfig.VolumeMounts), + VolumeMounts: data.volumeMounts, Ports: physicalPortsToCreateContainerPorts(containerConfig.Ports), Networks: data.networks, Env: containerConfig.Env, @@ -1170,9 +1260,10 @@ func (r *PhysicalContainerReconciler) ensurePhysicalContainerNetworkConnections( desiredConnections = make(map[string]apiv2.PhysicalContainerNetworkConnectionSpec, len(container.Spec.Container.Networks)) for i := range container.Spec.Container.Networks { network := &container.Spec.Container.Networks[i] + networkName := commonapi.AsNamespacedName(network.Name, container.Namespace) desiredConnections[physicalContainerNetworkConnectionName(container, i)] = apiv2.PhysicalContainerNetworkConnectionSpec{ ContainerRef: container.Name, - NetworkRef: network.Name, + NetworkRef: networkName.Name, Aliases: append([]string{}, network.Aliases...), } } @@ -1354,19 +1445,6 @@ func physicalPortsToCreateContainerPorts(ports []apiv2.ContainerPort) []containe return retval } -func physicalVolumeMountsToCreateContainerVolumeMounts(mounts []apiv2.VolumeMount) []containers.CreateContainerVolumeMount { - retval := make([]containers.CreateContainerVolumeMount, len(mounts)) - for i, mount := range mounts { - retval[i] = containers.CreateContainerVolumeMount{ - Type: containers.VolumeMountType(mount.Type), - Source: mount.Source, - Target: mount.Target, - ReadOnly: mount.ReadOnly, - } - } - return retval -} - func physicalContainerCreationLabels(container *apiv2.PhysicalContainer, log logr.Logger) []containers.Label { return physicalResourceCreationLabels( container.Spec.Container.Labels, diff --git a/controllers/physical_container_controller_test.go b/controllers/physical_container_controller_test.go new file mode 100644 index 00000000..4daf465f --- /dev/null +++ b/controllers/physical_container_controller_test.go @@ -0,0 +1,104 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See LICENSE in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +package controllers + +import ( + "context" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + apiv2 "github.com/microsoft/dcp/api/v2" +) + +func TestPhysicalContainerDependenciesRejectTerminatingResources(t *testing.T) { + t.Parallel() + + ctx := context.Background() + scheme := runtime.NewScheme() + require.NoError(t, apiv2.AddToScheme(scheme)) + deletionTimestamp := metav1.Now() + finalizers := []string{"test-finalizer"} + + image := &apiv2.PhysicalContainerImage{ + ObjectMeta: metav1.ObjectMeta{ + Name: "image", + Namespace: "namespace", + DeletionTimestamp: &deletionTimestamp, + Finalizers: finalizers, + }, + Status: apiv2.PhysicalContainerImageStatus{ + Phase: apiv2.PhysicalContainerImagePhaseReady, + ImageID: "image-id", + }, + } + volume := &apiv2.PhysicalContainerVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "volume", + Namespace: "namespace", + DeletionTimestamp: &deletionTimestamp, + Finalizers: finalizers, + }, + Status: apiv2.PhysicalContainerVolumeStatus{ + Phase: apiv2.PhysicalContainerVolumePhaseReady, + VolumeID: "volume-id", + }, + } + network := &apiv2.PhysicalContainerNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "network", + Namespace: "namespace", + DeletionTimestamp: &deletionTimestamp, + Finalizers: finalizers, + }, + Status: apiv2.PhysicalContainerNetworkStatus{ + Phase: apiv2.PhysicalContainerNetworkPhaseReady, + NetworkID: "network-id", + }, + } + client := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(image, volume, network). + Build() + reconciler := &PhysicalContainerReconciler{ + ReconcilerBase: NewReconcilerBase[apiv2.PhysicalContainer](client, client, logr.Discard(), ctx), + } + container := &apiv2.PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{Name: "container", Namespace: "namespace"}, + Spec: apiv2.PhysicalContainerSpec{ + Container: &apiv2.PhysicalContainerConfig{ + ImageRef: image.Name, + VolumeMounts: []apiv2.VolumeMount{{ + Type: apiv2.NamedVolumeMount, + VolumeRef: volume.Name, + Target: "/data", + }}, + Networks: []apiv2.ContainerNetworkConnectionConfig{{ + Name: network.Name, + }}, + }, + }, + } + + imageReady, _, imageProgress, imageMessage, _ := reconciler.resolvePhysicalContainerImage(ctx, container, logr.Discard()) + require.False(t, imageReady) + require.Equal(t, physicalResourceProgressNotReady, imageProgress) + require.Equal(t, `PhysicalContainerImage "image" is terminating.`, imageMessage) + + volumesReady, _, volumeProgress, volumeMessage := reconciler.resolvePhysicalContainerVolumes(ctx, container, logr.Discard()) + require.False(t, volumesReady) + require.Equal(t, physicalResourceProgressNotReady, volumeProgress) + require.Equal(t, `PhysicalContainerVolume "volume" is terminating.`, volumeMessage) + + networksReady, _, networkProgress, networkMessage := reconciler.resolvePhysicalContainerNetworks(ctx, container, logr.Discard()) + require.False(t, networksReady) + require.Equal(t, physicalResourceProgressNotReady, networkProgress) + require.Equal(t, `PhysicalContainerNetwork "network" is terminating.`, networkMessage) +} diff --git a/controllers/physical_container_data.go b/controllers/physical_container_data.go index 137ca482..73403c72 100644 --- a/controllers/physical_container_data.go +++ b/controllers/physical_container_data.go @@ -6,6 +6,7 @@ package controllers import ( + std_slices "slices" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -23,6 +24,7 @@ const ( physicalContainerStateNamespace physicalContainerState = iota + 1 physicalContainerStateResolve physicalContainerStateImage + physicalContainerStateVolumes physicalContainerStateNetworks physicalContainerStateCreate physicalContainerStateReplace @@ -58,6 +60,9 @@ type physicalContainerData struct { // Image name resolved from the referenced PhysicalContainerImage. image string + // Runtime volume mounts resolved from the referenced PhysicalContainerVolumes. + volumeMounts []containers.CreateContainerVolumeMount + // Runtime networks resolved from the referenced PhysicalContainerNetworks. networks []containers.CreateContainerNetworkOptions @@ -119,6 +124,7 @@ func (data *physicalContainerData) Clone() *physicalContainerData { progress: data.progress, containerID: data.containerID, image: data.image, + volumeMounts: append([]containers.CreateContainerVolumeMount{}, data.volumeMounts...), networks: clonePhysicalContainerNetworks(data.networks), failureMessage: data.failureMessage, portMappingFailureMessage: data.portMappingFailureMessage, @@ -149,6 +155,10 @@ func (data *physicalContainerData) UpdateFrom(other *physicalContainerData) bool data.image = other.image updated = true } + if !std_slices.Equal(data.volumeMounts, other.volumeMounts) { + data.volumeMounts = append([]containers.CreateContainerVolumeMount{}, other.volumeMounts...) + updated = true + } if !physicalContainerNetworksEqual(data.networks, other.networks) { data.networks = clonePhysicalContainerNetworks(other.networks) updated = true @@ -259,6 +269,16 @@ var physicalContainerProjections = physicalResourceProjectionTable[physicalConta phase: apiv2.PhysicalContainerPhaseUnknown, conditionStatus: metav1.ConditionFalse, conditionReason: apiv2.PhysicalContainerReasonImageLookupFailed, requeue: true, requeueDelay: LongDelay, }, + {state: physicalContainerStateVolumes, progress: physicalResourceProgressNotFound}: { + phase: apiv2.PhysicalContainerPhasePending, conditionStatus: metav1.ConditionFalse, conditionReason: apiv2.PhysicalContainerReasonVolumeNotFound, + }, + {state: physicalContainerStateVolumes, progress: physicalResourceProgressNotReady}: { + phase: apiv2.PhysicalContainerPhasePending, conditionStatus: metav1.ConditionFalse, conditionReason: apiv2.PhysicalContainerReasonVolumeNotReady, + }, + {state: physicalContainerStateVolumes, progress: physicalResourceProgressRetryPending}: { + phase: apiv2.PhysicalContainerPhaseUnknown, conditionStatus: metav1.ConditionFalse, conditionReason: apiv2.PhysicalContainerReasonVolumeLookupFailed, + requeue: true, requeueDelay: LongDelay, + }, {state: physicalContainerStateNetworks, progress: physicalResourceProgressNotFound}: { phase: apiv2.PhysicalContainerPhasePending, conditionStatus: metav1.ConditionFalse, conditionReason: apiv2.PhysicalContainerReasonNetworkNotFound, }, diff --git a/controllers/physical_container_network_controller.go b/controllers/physical_container_network_controller.go index 65864fc3..4e67baac 100644 --- a/controllers/physical_container_network_controller.go +++ b/controllers/physical_container_network_controller.go @@ -26,6 +26,7 @@ import ( apiv2 "github.com/microsoft/dcp/api/v2" "github.com/microsoft/dcp/internal/containers" + "github.com/microsoft/dcp/pkg/commonapi" "github.com/microsoft/dcp/pkg/resiliency" ) @@ -90,7 +91,8 @@ func (r *PhysicalContainerNetworkReconciler) SetupWithManager(mgr ctrl.Manager, if connection.Spec.ContainerRef == "" { return nil } - return []string{connection.Spec.ContainerRef} + containerName := commonapi.AsNamespacedName(connection.Spec.ContainerRef, connection.Namespace) + return []string{containerName.Name} }, ) if containerRefIndexErr != nil { @@ -106,7 +108,8 @@ func (r *PhysicalContainerNetworkReconciler) SetupWithManager(mgr ctrl.Manager, if connection.Spec.NetworkRef == "" { return nil } - return []string{connection.Spec.NetworkRef} + networkName := commonapi.AsNamespacedName(connection.Spec.NetworkRef, connection.Namespace) + return []string{networkName.Name} }, ) if networkRefIndexErr != nil { @@ -133,10 +136,7 @@ func (r *PhysicalContainerNetworkReconciler) networkForPhysicalContainerNetworkC return nil } return []reconcile.Request{{ - NamespacedName: types.NamespacedName{ - Namespace: connection.Namespace, - Name: connection.Spec.NetworkRef, - }, + NamespacedName: commonapi.AsNamespacedName(connection.Spec.NetworkRef, connection.Namespace), }} } @@ -156,22 +156,20 @@ func (r *PhysicalContainerNetworkReconciler) networksForPhysicalContainer( return nil } - networkNames := make(map[string]struct{}, len(connections.Items)) + networkNames := make(map[types.NamespacedName]struct{}, len(connections.Items)) requests := make([]reconcile.Request, 0, len(connections.Items)) for i := range connections.Items { - networkName := connections.Items[i].Spec.NetworkRef - if networkName == "" { + networkRef := connections.Items[i].Spec.NetworkRef + if networkRef == "" { continue } + networkName := commonapi.AsNamespacedName(networkRef, connections.Items[i].Namespace) if _, found := networkNames[networkName]; found { continue } networkNames[networkName] = struct{}{} requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: networkName, - }, + NamespacedName: networkName, }) } return requests @@ -1159,7 +1157,7 @@ func (r *PhysicalContainerNetworkReconciler) ensurePhysicalContainerNetworkConne for i := range connections.Items { connection := &connections.Items[i] container := apiv2.PhysicalContainer{} - containerName := types.NamespacedName{Namespace: connection.Namespace, Name: connection.Spec.ContainerRef} + containerName := commonapi.AsNamespacedName(connection.Spec.ContainerRef, connection.Namespace) containerLookupErr := r.Get(ctx, containerName, &container) switch { case apierrors.IsNotFound(containerLookupErr): diff --git a/controllers/physical_resource_projection_test.go b/controllers/physical_resource_projection_test.go index eadd64f6..315b4b57 100644 --- a/controllers/physical_resource_projection_test.go +++ b/controllers/physical_resource_projection_test.go @@ -33,6 +33,7 @@ func TestPhysicalResourceStateStringsDistinguishUnsetAndInvalidValues(t *testing require.Equal(t, "physicalResourceProgress(999)", physicalResourceProgress(999).String()) require.Equal(t, "Unset", physicalContainerState(0).String()) + require.Equal(t, "Volumes", physicalContainerStateVolumes.String()) require.Equal(t, "physicalContainerState(999)", physicalContainerState(999).String()) require.Equal(t, "Unset", physicalContainerImageState(0).String()) require.Equal(t, "physicalContainerImageState(999)", physicalContainerImageState(999).String()) diff --git a/controllers/physical_resource_state_string.go b/controllers/physical_resource_state_string.go index dc301305..d296b82d 100644 --- a/controllers/physical_resource_state_string.go +++ b/controllers/physical_resource_state_string.go @@ -66,6 +66,8 @@ func (state physicalContainerState) String() string { return "Resolve" case physicalContainerStateImage: return "Image" + case physicalContainerStateVolumes: + return "Volumes" case physicalContainerStateCreate: return "Create" case physicalContainerStateReplace: diff --git a/internal/dcpclient/client.go b/internal/dcpclient/client.go index 7519e90d..19076812 100644 --- a/internal/dcpclient/client.go +++ b/internal/dcpclient/client.go @@ -130,7 +130,7 @@ func ApplyDcpOptions(config *clientgorest.Config) { // need a workload ID to scope reuse and orphan reaping the way V1 persistent containers do through // the state store. Until that lands, apiv2.NamespaceWorkloadIDAnnotation is validated but has no // effect on behavior. See the "Physical resource layer" roadmap item covering crash cleanup in -// docs/v2-resource-plan.md. +// plan/v2-resource-plan.md. func ResolveNamespaceWorkloadID( ctx context.Context, reader ctrl_client.Reader, diff --git a/pkg/generated/openapi/zz_generated.openapi.go b/pkg/generated/openapi/zz_generated.openapi.go index fa8f6d9f..b3669a12 100644 --- a/pkg/generated/openapi/zz_generated.openapi.go +++ b/pkg/generated/openapi/zz_generated.openapi.go @@ -4372,7 +4372,7 @@ func schema_microsoft_dcp_api_v2_ContainerNetworkConnectionConfig(ref common.Ref Properties: map[string]spec.Schema{ "name": { SchemaProps: spec.SchemaProps{ - Description: "Name of the PhysicalContainerNetwork to connect to in the container's namespace.", + Description: "Name identifies a PhysicalContainerNetwork in the same namespace using or /. Cross-namespace references are not supported.", Default: "", Type: []string{"string"}, Format: "", @@ -4810,7 +4810,7 @@ func schema_microsoft_dcp_api_v2_PhysicalContainerConfig(ref common.ReferenceCal }, "imageRef": { SchemaProps: spec.SchemaProps{ - Description: "ImageRef is the name of a PhysicalContainerImage in the same namespace to use when creating a new runtime container.", + Description: "ImageRef identifies a PhysicalContainerImage in the same namespace using or /. Cross-namespace references are not supported.", Type: []string{"string"}, Format: "", }, @@ -5489,7 +5489,7 @@ func schema_microsoft_dcp_api_v2_PhysicalContainerNetworkConnectionSpec(ref comm Properties: map[string]spec.Schema{ "containerRef": { SchemaProps: spec.SchemaProps{ - Description: "ContainerRef is the name of the PhysicalContainer to connect.", + Description: "ContainerRef identifies the PhysicalContainer using or /. Cross-namespace references are not supported.", Default: "", Type: []string{"string"}, Format: "", @@ -5497,7 +5497,7 @@ func schema_microsoft_dcp_api_v2_PhysicalContainerNetworkConnectionSpec(ref comm }, "networkRef": { SchemaProps: spec.SchemaProps{ - Description: "NetworkRef is the name of the PhysicalContainerNetwork to connect to.", + Description: "NetworkRef identifies the PhysicalContainerNetwork using or /. Cross-namespace references are not supported.", Default: "", Type: []string{"string"}, Format: "", @@ -6488,8 +6488,14 @@ func schema_microsoft_dcp_api_v2_VolumeMount(ref common.ReferenceCallback) commo }, "source": { SchemaProps: spec.SchemaProps{ - Description: "Bind mounts: the host directory to mount. Volume mounts: name of the volume to mount.", - Default: "", + Description: "Source is the host directory to mount for bind mounts.", + Type: []string{"string"}, + Format: "", + }, + }, + "volumeRef": { + SchemaProps: spec.SchemaProps{ + Description: "VolumeRef identifies a PhysicalContainerVolume in the same namespace using or /. Cross-namespace references are not supported.", Type: []string{"string"}, Format: "", }, @@ -6510,7 +6516,7 @@ func schema_microsoft_dcp_api_v2_VolumeMount(ref common.ReferenceCallback) commo }, }, }, - Required: []string{"type", "source", "target"}, + Required: []string{"type", "target"}, }, }, } diff --git a/docs/v2-resource-plan.md b/plan/v2-resource-plan.md similarity index 96% rename from docs/v2-resource-plan.md rename to plan/v2-resource-plan.md index f4c52a50..9965af6c 100644 --- a/docs/v2-resource-plan.md +++ b/plan/v2-resource-plan.md @@ -77,7 +77,7 @@ This document tracks the intended direction for DCP V2 resources. The current V2 - `Namespace` defines the namespace boundary for V2 resources and provides namespace-scoped cleanup. Namespace deletion waits for admitted child storage creates to finish, even if their HTTP requests time out, and rejects new creates until cleanup completes. Namespace collection deletion is unsupported. Top-level V2 mutation dry-runs are rejected because the storage backend does not provide no-write dry-run semantics. - `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 a same-namespace `PhysicalContainerImage`. +- `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. - `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. @@ -87,26 +87,21 @@ This document tracks the intended direction for DCP V2 resources. The current V2 ### Physical resource layer -1. Update `PhysicalContainer` to use physical network and volume resources. - - Replace direct runtime network names with references to same-namespace `PhysicalContainerNetwork` resources where appropriate. - - Replace direct runtime volume names with references to same-namespace `PhysicalContainerVolume` resources where appropriate. - - Watch referenced network and volume resources so containers reconcile when dependencies become ready. - -2. Decide how monitor processes should clean up physical resources after DCP crashes. +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. -3. Migrate V1 container-network tunnel proxy to V2 physical resources. +2. 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. -4. Migrate V1 container resource lifecycle to V2 physical resources. +3. 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. -5. Add logical resource controllers. +4. 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 d53b6f57..aeff4498 100644 --- a/test/integration/v2_physical_container_controller_test.go +++ b/test/integration/v2_physical_container_controller_test.go @@ -150,7 +150,7 @@ func TestV2PhysicalContainerControllerReconcilesWhenReferencedImageBecomesReady( Name: "watched-image-container", Namespace: namespace.Name, }, - Spec: apiv2.PhysicalContainerSpec{Container: &apiv2.PhysicalContainerConfig{ImageRef: imageName, + Spec: apiv2.PhysicalContainerSpec{Container: &apiv2.PhysicalContainerConfig{ImageRef: namespace.Name + "/" + imageName, ContainerName: containerName}, }, } @@ -190,6 +190,199 @@ func TestV2PhysicalContainerControllerReconcilesWhenReferencedImageBecomesReady( require.Equal(t, 1, containerOrchestrator.CreateContainerCallCount(containerName)) } +func TestV2PhysicalContainerControllerCreatesContainerWithVolumes(t *testing.T) { + t.Parallel() + ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) + defer cancel() + + namespace := createActiveV2Namespace(t, ctx, "v2-pctr-volumes") + image := createReadyV2PhysicalContainerImage(t, ctx, namespace.Name, "volume-image", "volume-image") + runtimeVolumeName := "v2-pctr-volume-runtime" + require.NoError(t, containerOrchestrator.CreateVolume(ctx, containers.CreateVolumeOptions{Name: runtimeVolumeName})) + removeRuntimeVolumeOnCleanup(t, runtimeVolumeName) + volume := &apiv2.PhysicalContainerVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: "container-volume", + Namespace: namespace.Name, + }, + Spec: apiv2.PhysicalContainerVolumeSpec{VolumeID: runtimeVolumeName}, + } + require.NoError(t, client.Create(ctx, volume)) + waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhaseReady) + + container := &apiv2.PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "volume-container", + Namespace: namespace.Name, + }, + Spec: apiv2.PhysicalContainerSpec{ + Container: &apiv2.PhysicalContainerConfig{ + ImageRef: image.Name, + ContainerName: "v2-pctr-volume-container", + VolumeMounts: []apiv2.VolumeMount{ + {Type: apiv2.NamedVolumeMount, VolumeRef: volume.Name, Target: "/volume", ReadOnly: true}, + {Type: apiv2.BindMount, Source: "/host/data", Target: "/bind"}, + }, + }, + }, + } + require.NoError(t, client.Create(ctx, container)) + + updatedContainer := waitPhysicalContainerPhase(t, ctx, container.NamespacedName(), apiv2.PhysicalContainerPhaseRunning) + removeRuntimeContainerOnCleanup(t, updatedContainer.Status.ContainerID) + + inspectedContainers, inspectErr := containerOrchestrator.InspectContainers(ctx, containers.InspectContainersOptions{ + Containers: []string{updatedContainer.Status.ContainerID}, + }) + require.NoError(t, inspectErr) + require.Len(t, inspectedContainers, 1) + require.Equal(t, []containers.VolumeMount{ + {Type: containers.NamedVolumeMount, Source: runtimeVolumeName, Target: "/volume", ReadOnly: true}, + {Type: containers.BindMount, Source: "/host/data", Target: "/bind"}, + }, inspectedContainers[0].Mounts) +} + +func TestV2PhysicalContainerControllerReconcilesWhenReferencedVolumeBecomesReady(t *testing.T) { + t.Parallel() + ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) + defer cancel() + + namespace := createActiveV2Namespace(t, ctx, "v2-pctr-volume-watch") + image := createReadyV2PhysicalContainerImage(t, ctx, namespace.Name, "watched-volume-image", "watched-volume-image") + volumeName := "watched-volume" + containerName := "v2-pctr-watched-volume-container" + container := &apiv2.PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "watched-volume-container", + Namespace: namespace.Name, + }, + Spec: apiv2.PhysicalContainerSpec{ + Container: &apiv2.PhysicalContainerConfig{ + ImageRef: image.Name, + ContainerName: containerName, + VolumeMounts: []apiv2.VolumeMount{{ + Type: apiv2.NamedVolumeMount, + VolumeRef: namespace.Name + "/" + volumeName, + Target: "/data", + }}, + }, + }, + } + require.NoError(t, client.Create(ctx, container)) + + pendingContainer := waitObjectAssumesState(t, ctx, container.NamespacedName(), func(currentContainer *apiv2.PhysicalContainer) (bool, error) { + readyCondition := apimeta.FindStatusCondition(currentContainer.Status.Conditions, string(apiv2.ConditionReady)) + return currentContainer.Status.Phase == apiv2.PhysicalContainerPhasePending && + readyCondition != nil && + apiv2.ConditionReason(readyCondition.Reason) == apiv2.PhysicalContainerReasonVolumeNotFound, nil + }) + requireReadyCondition(t, pendingContainer.Status.Conditions, metav1.ConditionFalse, apiv2.PhysicalContainerReasonVolumeNotFound) + require.Equal(t, 0, containerOrchestrator.CreateContainerCallCount(containerName)) + + runtimeVolumeName := "v2-pctr-watched-volume-runtime" + releaseCreate := containerOrchestrator.BlockCreateVolume(runtimeVolumeName) + defer releaseCreate() + removeRuntimeVolumeOnCleanup(t, runtimeVolumeName) + volume := &apiv2.PhysicalContainerVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: volumeName, + Namespace: namespace.Name, + }, + Spec: apiv2.PhysicalContainerVolumeSpec{ + Volume: &apiv2.PhysicalContainerVolumeConfig{VolumeName: runtimeVolumeName}, + }, + } + require.NoError(t, client.Create(ctx, volume)) + + pendingContainer = waitObjectAssumesState(t, ctx, container.NamespacedName(), func(currentContainer *apiv2.PhysicalContainer) (bool, error) { + readyCondition := apimeta.FindStatusCondition(currentContainer.Status.Conditions, string(apiv2.ConditionReady)) + return currentContainer.Status.Phase == apiv2.PhysicalContainerPhasePending && + readyCondition != nil && + apiv2.ConditionReason(readyCondition.Reason) == apiv2.PhysicalContainerReasonVolumeNotReady, nil + }) + requireReadyCondition(t, pendingContainer.Status.Conditions, metav1.ConditionFalse, apiv2.PhysicalContainerReasonVolumeNotReady) + require.Equal(t, 0, containerOrchestrator.CreateContainerCallCount(containerName)) + releaseCreate() + + updatedContainer := waitPhysicalContainerPhase(t, ctx, container.NamespacedName(), apiv2.PhysicalContainerPhaseRunning) + removeRuntimeContainerOnCleanup(t, updatedContainer.Status.ContainerID) + require.Equal(t, 1, containerOrchestrator.CreateContainerCallCount(containerName)) + + inspectedContainers, inspectErr := containerOrchestrator.InspectContainers(ctx, containers.InspectContainersOptions{ + Containers: []string{updatedContainer.Status.ContainerID}, + }) + require.NoError(t, inspectErr) + require.Len(t, inspectedContainers, 1) + require.Len(t, inspectedContainers[0].Mounts, 1) + require.Equal(t, runtimeVolumeName, inspectedContainers[0].Mounts[0].Source) +} + +func TestV2PhysicalContainerControllerRechecksEarlierDependencies(t *testing.T) { + t.Parallel() + ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) + defer cancel() + + namespace := createActiveV2Namespace(t, ctx, "v2-pctr-rechecks-dependencies") + image := createReadyV2PhysicalContainerImage(t, ctx, namespace.Name, "rechecked-image", "rechecked-image") + volumeName := "rechecked-volume" + containerName := "v2-pctr-rechecks-dependencies-container" + container := &apiv2.PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rechecks-dependencies-container", + Namespace: namespace.Name, + }, + Spec: apiv2.PhysicalContainerSpec{ + Container: &apiv2.PhysicalContainerConfig{ + ImageRef: image.Name, + ContainerName: containerName, + VolumeMounts: []apiv2.VolumeMount{{ + Type: apiv2.NamedVolumeMount, + VolumeRef: volumeName, + Target: "/data", + }}, + }, + }, + } + require.NoError(t, client.Create(ctx, container)) + + volumePendingContainer := waitObjectAssumesState(t, ctx, container.NamespacedName(), func(currentContainer *apiv2.PhysicalContainer) (bool, error) { + readyCondition := apimeta.FindStatusCondition(currentContainer.Status.Conditions, string(apiv2.ConditionReady)) + return readyCondition != nil && + apiv2.ConditionReason(readyCondition.Reason) == apiv2.PhysicalContainerReasonVolumeNotFound, nil + }) + requireReadyCondition(t, volumePendingContainer.Status.Conditions, metav1.ConditionFalse, apiv2.PhysicalContainerReasonVolumeNotFound) + + restoreImageInspection := simulateV2PhysicalContainerImageUnavailable(t, ctx, image) + + imagePendingContainer := waitObjectAssumesState(t, ctx, container.NamespacedName(), func(currentContainer *apiv2.PhysicalContainer) (bool, error) { + readyCondition := apimeta.FindStatusCondition(currentContainer.Status.Conditions, string(apiv2.ConditionReady)) + return readyCondition != nil && + apiv2.ConditionReason(readyCondition.Reason) == apiv2.PhysicalContainerReasonImageNotReady, nil + }) + requireReadyCondition(t, imagePendingContainer.Status.Conditions, metav1.ConditionFalse, apiv2.PhysicalContainerReasonImageNotReady) + require.Equal(t, 0, containerOrchestrator.CreateContainerCallCount(containerName)) + + restoreImageInspection() + waitPhysicalContainerImagePhase(t, ctx, image.NamespacedName(), apiv2.PhysicalContainerImagePhaseReady) + + runtimeVolumeName := "v2-pctr-rechecks-dependencies-volume" + require.NoError(t, containerOrchestrator.CreateVolume(ctx, containers.CreateVolumeOptions{Name: runtimeVolumeName})) + removeRuntimeVolumeOnCleanup(t, runtimeVolumeName) + volume := &apiv2.PhysicalContainerVolume{ + ObjectMeta: metav1.ObjectMeta{ + Name: volumeName, + Namespace: namespace.Name, + }, + Spec: apiv2.PhysicalContainerVolumeSpec{VolumeID: runtimeVolumeName}, + } + require.NoError(t, client.Create(ctx, volume)) + waitPhysicalContainerVolumePhase(t, ctx, volume.NamespacedName(), apiv2.PhysicalContainerVolumePhaseReady) + + updatedContainer := waitPhysicalContainerPhase(t, ctx, container.NamespacedName(), apiv2.PhysicalContainerPhaseRunning) + removeRuntimeContainerOnCleanup(t, updatedContainer.Status.ContainerID) + require.Equal(t, 1, containerOrchestrator.CreateContainerCallCount(containerName)) +} + func TestV2PhysicalContainerControllerCreatesContainerWithNetworks(t *testing.T) { t.Parallel() ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) @@ -268,7 +461,7 @@ func TestV2PhysicalContainerControllerWaitsForNetwork(t *testing.T) { ImageRef: image.Name, ContainerName: containerName, Networks: []apiv2.ContainerNetworkConnectionConfig{ - {Name: networkName}, + {Name: namespace.Name + "/" + networkName}, }, }, }, @@ -592,6 +785,54 @@ func TestV2PhysicalContainerControllerRetriesCreateWithoutPartialContainer(t *te require.Equal(t, 0, containerOrchestrator.RemoveContainerCallCount(containerName)) } +func TestV2PhysicalContainerControllerRepreparesDependenciesBeforeCreateRetry(t *testing.T) { + t.Parallel() + ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) + defer cancel() + + namespace := createActiveV2Namespace(t, ctx, "v2-pctr-reprepare-create-retry") + image := createReadyV2PhysicalContainerImage(t, ctx, namespace.Name, "reprepare-image", "reprepare-image") + containerName := "v2-pctr-reprepare-create-retry-container" + containerOrchestrator.FailNextCreateContainer(containerName, errors.New("create failed once")) + + container := &apiv2.PhysicalContainer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "reprepare-create-retry-container", + Namespace: namespace.Name, + }, + Spec: apiv2.PhysicalContainerSpec{Container: &apiv2.PhysicalContainerConfig{ + ImageRef: image.Name, + ContainerName: containerName, + }}, + } + require.NoError(t, client.Create(ctx, container)) + + retryPendingContainer := waitObjectAssumesState(t, ctx, container.NamespacedName(), func(currentContainer *apiv2.PhysicalContainer) (bool, error) { + readyCondition := apimeta.FindStatusCondition(currentContainer.Status.Conditions, string(apiv2.ConditionReady)) + return readyCondition != nil && + apiv2.ConditionReason(readyCondition.Reason) == apiv2.PhysicalContainerReasonCreateFailed, nil + }) + requireReadyCondition(t, retryPendingContainer.Status.Conditions, metav1.ConditionFalse, apiv2.PhysicalContainerReasonCreateFailed) + require.Equal(t, 1, containerOrchestrator.CreateContainerCallCount(containerName)) + + restoreImageInspection := simulateV2PhysicalContainerImageUnavailable(t, ctx, image) + + imagePendingContainer := waitObjectAssumesState(t, ctx, container.NamespacedName(), func(currentContainer *apiv2.PhysicalContainer) (bool, error) { + readyCondition := apimeta.FindStatusCondition(currentContainer.Status.Conditions, string(apiv2.ConditionReady)) + return readyCondition != nil && + apiv2.ConditionReason(readyCondition.Reason) == apiv2.PhysicalContainerReasonImageNotReady, nil + }) + requireReadyCondition(t, imagePendingContainer.Status.Conditions, metav1.ConditionFalse, apiv2.PhysicalContainerReasonImageNotReady) + require.Equal(t, 1, containerOrchestrator.CreateContainerCallCount(containerName)) + + restoreImageInspection() + waitPhysicalContainerImagePhase(t, ctx, image.NamespacedName(), apiv2.PhysicalContainerImagePhaseReady) + + updatedContainer := waitPhysicalContainerPhase(t, ctx, container.NamespacedName(), apiv2.PhysicalContainerPhaseRunning) + removeRuntimeContainerOnCleanup(t, updatedContainer.Status.ContainerID) + require.Equal(t, 2, containerOrchestrator.CreateContainerCallCount(containerName)) +} + func TestV2PhysicalContainerControllerRetriesCreateAfterFailure(t *testing.T) { t.Parallel() ctx, cancel := testutil.GetTestContext(t, defaultIntegrationTestTimeout) @@ -1224,6 +1465,26 @@ func waitCreateContainerCallCount(t *testing.T, ctx context.Context, name string require.NoError(t, waitErr) } +func simulateV2PhysicalContainerImageUnavailable( + t *testing.T, + ctx context.Context, + image *apiv2.PhysicalContainerImage, +) func() { + t.Helper() + + restoreImageInspection := containerOrchestrator.FailInspectImage(image.Status.ImageID, containers.ErrNotFound) + t.Cleanup(restoreImageInspection) + require.NoError(t, retryOnConflict[apiv2.PhysicalContainerImage](ctx, image.NamespacedName(), func(ctx context.Context, currentImage *apiv2.PhysicalContainerImage) error { + if currentImage.Annotations == nil { + currentImage.Annotations = map[string]string{} + } + currentImage.Annotations["test.dcp.microsoft.com/reconcile"] = "image-unavailable" + return client.Update(ctx, currentImage) + })) + waitPhysicalContainerImagePhase(t, ctx, image.NamespacedName(), apiv2.PhysicalContainerImagePhaseUnknown) + return restoreImageInspection +} + func createActiveV2Namespace(t *testing.T, ctx context.Context, name string) *apiv2.Namespace { t.Helper() diff --git a/test/integration/v2_physical_container_network_connection_test.go b/test/integration/v2_physical_container_network_connection_test.go index f796b95a..bdc6c0bb 100644 --- a/test/integration/v2_physical_container_network_connection_test.go +++ b/test/integration/v2_physical_container_network_connection_test.go @@ -51,8 +51,8 @@ func TestV2PhysicalContainerNetworkConnectionReconcilesMembership(t *testing.T) connection := &apiv2.PhysicalContainerNetworkConnection{ ObjectMeta: metav1.ObjectMeta{Name: "connection", Namespace: namespace.Name}, Spec: apiv2.PhysicalContainerNetworkConnectionSpec{ - ContainerRef: physicalContainer.Name, - NetworkRef: physicalNetwork.Name, + ContainerRef: namespace.Name + "/" + physicalContainer.Name, + NetworkRef: namespace.Name + "/" + physicalNetwork.Name, Aliases: []string{"physical-alias"}, }, }