diff --git a/api/v1alpha1/port_types.go b/api/v1alpha1/port_types.go index d54f3cf95..af07d869b 100644 --- a/api/v1alpha1/port_types.go +++ b/api/v1alpha1/port_types.go @@ -202,7 +202,13 @@ type PortResourceSpec struct { MACAddress string `json:"macAddress,omitempty"` // hostID specifies the host where the port will be bound. + // Note that when the port is attached to a server, OpenStack may + // rebind the port to the server's actual compute host, which may + // differ from the specified hostID if no matching scheduler hint + // is used. In this case the port's status will reflect the actual + // binding host, not the value specified here. // +optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="hostID is immutable" HostID *HostID `json:"hostID,omitempty"` } diff --git a/cmd/models-schema/zz_generated.openapi.go b/cmd/models-schema/zz_generated.openapi.go index 285618a39..f47211048 100644 --- a/cmd/models-schema/zz_generated.openapi.go +++ b/cmd/models-schema/zz_generated.openapi.go @@ -4959,7 +4959,7 @@ func schema_openstack_resource_controller_v2_api_v1alpha1_PortResourceSpec(ref c }, "hostID": { SchemaProps: spec.SchemaProps{ - Description: "hostID specifies the host where the port will be bound.", + Description: "hostID specifies the host where the port will be bound. Note that when the port is attached to a server, OpenStack may rebind the port to the server's actual compute host, which may differ from the specified hostID if no matching scheduler hint is used. In this case the port's status will reflect the actual binding host, not the value specified here.", Ref: ref("github.com/k-orc/openstack-resource-controller/v2/api/v1alpha1.HostID"), }, }, diff --git a/config/crd/bases/openstack.k-orc.cloud_ports.yaml b/config/crd/bases/openstack.k-orc.cloud_ports.yaml index d4e4999b7..7ea7190a9 100644 --- a/config/crd/bases/openstack.k-orc.cloud_ports.yaml +++ b/config/crd/bases/openstack.k-orc.cloud_ports.yaml @@ -300,8 +300,13 @@ spec: minLength: 1 type: string hostID: - description: hostID specifies the host where the port will be - bound. + description: |- + hostID specifies the host where the port will be bound. + Note that when the port is attached to a server, OpenStack may + rebind the port to the server's actual compute host, which may + differ from the specified hostID if no matching scheduler hint + is used. In this case the port's status will reflect the actual + binding host, not the value specified here. maxProperties: 1 minProperties: 1 properties: @@ -322,6 +327,8 @@ spec: type: string type: object x-kubernetes-validations: + - message: hostID is immutable + rule: self == oldSelf - message: exactly one of id or serverRef must be set rule: (has(self.id) && size(self.id) > 0) != (has(self.serverRef) && size(self.serverRef) > 0) diff --git a/internal/controllers/port/actuator.go b/internal/controllers/port/actuator.go index ecfcd7f93..363ae931d 100644 --- a/internal/controllers/port/actuator.go +++ b/internal/controllers/port/actuator.go @@ -377,14 +377,6 @@ func (actuator portActuator) updateResource(ctx context.Context, obj orcObjectPT reconcileStatus := progress.NewReconcileStatus(). WithReconcileStatus(secGroupDepRS) - // Resolve hostID if specified - var resolvedHostID string - if resource.HostID != nil { - var hostIDReconcileStatus progress.ReconcileStatus - resolvedHostID, hostIDReconcileStatus = resolveHostID(ctx, actuator.k8sClient, obj, resource.HostID) - reconcileStatus = reconcileStatus.WithReconcileStatus(hostIDReconcileStatus) - } - needsReschedule, _ := reconcileStatus.NeedsReschedule() if needsReschedule { return reconcileStatus @@ -403,7 +395,7 @@ func (actuator portActuator) updateResource(ctx context.Context, obj orcObjectPT updateOpts = baseUpdateOpts } - updateOpts = handlePortBindingUpdate(updateOpts, resource, osResource, resolvedHostID) + updateOpts = handlePortBindingUpdate(updateOpts, resource, osResource) updateOpts = handlePortSecurityUpdate(updateOpts, resource, osResource) needsUpdate, err := needsUpdate(updateOpts) @@ -529,7 +521,7 @@ func handleSecurityGroupRefsUpdate(updateOpts *ports.UpdateOpts, resource *resou } } -func handlePortBindingUpdate(updateOpts ports.UpdateOptsBuilder, resource *resourceSpecT, osResource *osResourceT, resolvedHostID string) ports.UpdateOptsBuilder { +func handlePortBindingUpdate(updateOpts ports.UpdateOptsBuilder, resource *resourceSpecT, osResource *osResourceT) ports.UpdateOptsBuilder { if resource.VNICType != "" { if resource.VNICType != osResource.VNICType { updateOpts = &portsbinding.UpdateOptsExt{ @@ -539,15 +531,6 @@ func handlePortBindingUpdate(updateOpts ports.UpdateOptsBuilder, resource *resou } } - if resolvedHostID != "" { - if resolvedHostID != osResource.HostID { - updateOpts = &portsbinding.UpdateOptsExt{ - UpdateOptsBuilder: updateOpts, - HostID: &resolvedHostID, - } - } - } - return updateOpts } diff --git a/internal/controllers/port/actuator_test.go b/internal/controllers/port/actuator_test.go index d6f7186c0..81a2a7cc6 100644 --- a/internal/controllers/port/actuator_test.go +++ b/internal/controllers/port/actuator_test.go @@ -359,7 +359,7 @@ func TestHandlePortBindingUpdate(t *testing.T) { }, } - updateOpts := handlePortBindingUpdate(&ports.UpdateOpts{}, resource, osResource, "") + updateOpts := handlePortBindingUpdate(&ports.UpdateOpts{}, resource, osResource) got, _ := needsUpdate(updateOpts) if got != tt.expectChange { diff --git a/internal/controllers/port/tests/port-update/00-assert.yaml b/internal/controllers/port/tests/port-update/00-assert.yaml index 6ec7e451d..fef380932 100644 --- a/internal/controllers/port/tests/port-update/00-assert.yaml +++ b/internal/controllers/port/tests/port-update/00-assert.yaml @@ -6,10 +6,6 @@ resourceRefs: kind: port name: port-update ref: port - - apiVersion: openstack.k-orc.cloud/v1alpha1 - kind: port - name: port-update-admin - ref: portAdmin assertAll: - celExpr: "port.status.id != ''" - celExpr: "port.status.resource.createdAt != ''" @@ -17,9 +13,6 @@ assertAll: - celExpr: "port.status.resource.macAddress != ''" - celExpr: "!has(port.status.resource.fixedIPs)" - celExpr: "!has(port.status.resource.description)" - # Following the network API reference, the default value for - # hostID field is an empty string. - - celExpr: "portAdmin.status.resource.hostID == ''" --- apiVersion: openstack.k-orc.cloud/v1alpha1 kind: Port @@ -43,26 +36,3 @@ status: message: OpenStack resource is up to date status: "False" reason: Success ---- -apiVersion: openstack.k-orc.cloud/v1alpha1 -kind: Port -metadata: - name: port-update-admin -status: - resource: - name: port-update-admin - adminStateUp: true - portSecurityEnabled: true - propagateUplinkStatus: false - revisionNumber: 1 - status: DOWN - vnicType: normal - conditions: - - type: Available - message: OpenStack resource is available - status: "True" - reason: Success - - type: Progressing - message: OpenStack resource is up to date - status: "False" - reason: Success diff --git a/internal/controllers/port/tests/port-update/00-minimal-resource.yaml b/internal/controllers/port/tests/port-update/00-minimal-resource.yaml index 03bbe59c3..d1242e77f 100644 --- a/internal/controllers/port/tests/port-update/00-minimal-resource.yaml +++ b/internal/controllers/port/tests/port-update/00-minimal-resource.yaml @@ -12,17 +12,3 @@ spec: portSecurity: Disabled # Need to set the default values to revert them correctly in the 02-revert-resource step. vnicType: normal ---- -# This port is intended to be used only to test fields editable -# by admin users -apiVersion: openstack.k-orc.cloud/v1alpha1 -kind: Port -metadata: - name: port-update-admin -spec: - cloudCredentialsRef: - cloudName: openstack-admin - secretName: openstack-clouds - managementPolicy: managed - resource: - networkRef: port-update diff --git a/internal/controllers/port/tests/port-update/01-assert.yaml b/internal/controllers/port/tests/port-update/01-assert.yaml index 1bcaf2d7f..c8f79187b 100644 --- a/internal/controllers/port/tests/port-update/01-assert.yaml +++ b/internal/controllers/port/tests/port-update/01-assert.yaml @@ -49,18 +49,3 @@ status: - type: Progressing status: "False" reason: Success ---- -apiVersion: openstack.k-orc.cloud/v1alpha1 -kind: Port -metadata: - name: port-update-admin -status: - resource: - hostID: devstack - conditions: - - type: Available - status: "True" - reason: Success - - type: Progressing - status: "False" - reason: Success diff --git a/internal/controllers/port/tests/port-update/01-updated-resource.yaml b/internal/controllers/port/tests/port-update/01-updated-resource.yaml index 2af467f7c..2ad77cb94 100644 --- a/internal/controllers/port/tests/port-update/01-updated-resource.yaml +++ b/internal/controllers/port/tests/port-update/01-updated-resource.yaml @@ -20,16 +20,3 @@ spec: - tag1 vnicType: direct portSecurity: Enabled ---- -apiVersion: openstack.k-orc.cloud/v1alpha1 -kind: Port -metadata: - name: port-update-admin -spec: - cloudCredentialsRef: - cloudName: openstack-admin - secretName: openstack-clouds - managementPolicy: managed - resource: - hostID: - id: devstack diff --git a/test/apivalidations/port_test.go b/test/apivalidations/port_test.go index 3c36dea83..61ef31e00 100644 --- a/test/apivalidations/port_test.go +++ b/test/apivalidations/port_test.go @@ -123,6 +123,20 @@ var _ = Describe("ORC Port API validations", func() { Expect(applyObj(ctx, port, patch)).To(MatchError(ContainSubstring("spec.resource.vnicType: Too long: may not be longer than 64"))) }) + It("should not allow hostID to be modified", func(ctx context.Context) { + port := portStub(namespace) + patch := basePortPatch(port) + patch.Spec.WithResource(applyconfigv1alpha1.PortResourceSpec(). + WithNetworkRef(networkName). + WithHostID(applyconfigv1alpha1.HostID().WithID("host-a"))) + Expect(applyObj(ctx, port, patch)).To(Succeed()) + + patch.Spec.WithResource(applyconfigv1alpha1.PortResourceSpec(). + WithNetworkRef(networkName). + WithHostID(applyconfigv1alpha1.HostID().WithID("host-b"))) + Expect(applyObj(ctx, port, patch)).To(MatchError(ContainSubstring("hostID is immutable"))) + }) + // Note: we can't create a test for when the portSecurity is set to Inherit and the securityGroupRefs are set, because // the validation is done in the OpenStack API and not in the ORC API. The OpenStack API will return an error if // the network has port security disabled and the port has security group references. diff --git a/website/docs/crd-reference.md b/website/docs/crd-reference.md index a62fbb623..2c2b9b8e8 100644 --- a/website/docs/crd-reference.md +++ b/website/docs/crd-reference.md @@ -2207,7 +2207,7 @@ _Appears in:_ | `portSecurity` _[PortSecurityState](#portsecuritystate)_ | portSecurity controls port security for this port.
When set to Enabled, port security is enabled.
When set to Disabled, port security is disabled and SecurityGroupRefs must be empty.
When set to Inherit (default), it takes the value from the network level. | Inherit | Enum: [Enabled Disabled Inherit]
| | `projectRef` _[KubernetesNameRef](#kubernetesnameref)_ | projectRef is a reference to the ORC Project this resource is associated with.
Typically, only used by admin. | | MaxLength: 253
MinLength: 1
| | `macAddress` _string_ | macAddress is the MAC address of the port. | | MaxLength: 32
| -| `hostID` _[HostID](#hostid)_ | hostID specifies the host where the port will be bound. | | MaxProperties: 1
MinProperties: 1
| +| `hostID` _[HostID](#hostid)_ | hostID specifies the host where the port will be bound.
Note that when the port is attached to a server, OpenStack may
rebind the port to the server's actual compute host, which may
differ from the specified hostID if no matching scheduler hint
is used. In this case the port's status will reflect the actual
binding host, not the value specified here. | | MaxProperties: 1
MinProperties: 1
| #### PortResourceStatus