diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index ae7b58fbd..2922eacdf 100644 --- a/.github/workflows/e2e.yaml +++ b/.github/workflows/e2e.yaml @@ -38,7 +38,7 @@ jobs: with: enable_workaround_docker_io: 'false' branch: ${{ matrix.openstack_version }} - enabled_services: "openstack-cli-server,neutron-trunk,neutron-port-trusted-vif" + enabled_services: "openstack-cli-server,neutron-trunk,neutron-port-trusted-vif,neutron-uplink-status-propagation" conf_overrides: | enable_plugin neutron https://github.com/openstack/neutron ${{ matrix.openstack_version }} enable_plugin manila https://github.com/openstack/manila ${{ matrix.openstack_version }} diff --git a/api/v1alpha1/port_types.go b/api/v1alpha1/port_types.go index f743acd16..108dc8c5f 100644 --- a/api/v1alpha1/port_types.go +++ b/api/v1alpha1/port_types.go @@ -244,6 +244,15 @@ type PortResourceSpec struct { // +optional // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="valueSpecs is immutable" ValueSpecs []PortValueSpec `json:"valueSpecs,omitempty"` + + // propagateUplinkStatus represents the uplink status propagation of + // the port. + // The field is now immutable due to a limitation on + // Dalmatian (2024.2) release, we should address this later. + // https://github.com/k-orc/openstack-resource-controller/pull/641#discussion_r2694783787 + // +optional + // +kubebuilder:validation:XValidation:rule="self == oldSelf",message="propagateUplinkStatus is immutable" + PropagateUplinkStatus *bool `json:"propagateUplinkStatus,omitempty"` } type PortResourceStatus struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 2f4fe34aa..25a947114 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -3386,6 +3386,11 @@ func (in *PortResourceSpec) DeepCopyInto(out *PortResourceSpec) { (*in)[i].DeepCopyInto(&(*out)[i]) } } + if in.PropagateUplinkStatus != nil { + in, out := &in.PropagateUplinkStatus, &out.PropagateUplinkStatus + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PortResourceSpec. diff --git a/cmd/models-schema/zz_generated.openapi.go b/cmd/models-schema/zz_generated.openapi.go index bbdaee5f0..c702e616c 100644 --- a/cmd/models-schema/zz_generated.openapi.go +++ b/cmd/models-schema/zz_generated.openapi.go @@ -6419,6 +6419,13 @@ func schema_openstack_resource_controller_v2_api_v1alpha1_PortResourceSpec(ref c }, }, }, + "propagateUplinkStatus": { + SchemaProps: spec.SchemaProps{ + Description: "propagateUplinkStatus represents the uplink status propagation of the port. The field is now immutable due to a limitation on Dalmatian (2024.2) release, we should address this later. https://github.com/k-orc/openstack-resource-controller/pull/641#discussion_r2694783787", + Type: []string{"boolean"}, + Format: "", + }, + }, }, Required: []string{"networkRef"}, }, diff --git a/config/crd/bases/openstack.k-orc.cloud_ports.yaml b/config/crd/bases/openstack.k-orc.cloud_ports.yaml index 8c38158c6..8212ec147 100644 --- a/config/crd/bases/openstack.k-orc.cloud_ports.yaml +++ b/config/crd/bases/openstack.k-orc.cloud_ports.yaml @@ -377,6 +377,17 @@ spec: x-kubernetes-validations: - message: projectRef is immutable rule: self == oldSelf + propagateUplinkStatus: + description: |- + propagateUplinkStatus represents the uplink status propagation of + the port. + The field is now immutable due to a limitation on + Dalmatian (2024.2) release, we should address this later. + https://github.com/k-orc/openstack-resource-controller/pull/641#discussion_r2694783787 + type: boolean + x-kubernetes-validations: + - message: propagateUplinkStatus is immutable + rule: self == oldSelf securityGroupRefs: description: |- securityGroupRefs are references to the security groups associated diff --git a/internal/controllers/port/actuator.go b/internal/controllers/port/actuator.go index cd4db8a5e..a602d2d69 100644 --- a/internal/controllers/port/actuator.go +++ b/internal/controllers/port/actuator.go @@ -249,13 +249,14 @@ func (actuator portActuator) CreateResource(ctx context.Context, obj *orcv1alpha } createOpts := ports.CreateOpts{ - NetworkID: *network.Status.ID, - Name: getResourceName(obj), - Description: string(ptr.Deref(resource.Description, "")), - ProjectID: projectID, - AdminStateUp: resource.AdminStateUp, - MACAddress: resource.MACAddress, - ValueSpecs: valueSpecs, + NetworkID: *network.Status.ID, + Name: getResourceName(obj), + Description: string(ptr.Deref(resource.Description, "")), + ProjectID: projectID, + AdminStateUp: resource.AdminStateUp, + MACAddress: resource.MACAddress, + ValueSpecs: valueSpecs, + PropagateUplinkStatus: resource.PropagateUplinkStatus, } if len(resource.AllowedAddressPairs) > 0 { diff --git a/internal/controllers/port/actuator_test.go b/internal/controllers/port/actuator_test.go index cf31483a9..84d12768f 100644 --- a/internal/controllers/port/actuator_test.go +++ b/internal/controllers/port/actuator_test.go @@ -462,7 +462,6 @@ func TestHandleTrustedVIFUpdate(t *testing.T) { } updateOpts := handlePortTrustedVIFUpdate(&ports.UpdateOpts{}, resource, osResource) - got, _ := needsUpdate(updateOpts) if got != tt.expectChange { t.Errorf("expected needsUpdate=%v, got %v", tt.expectChange, got) diff --git a/internal/controllers/port/status.go b/internal/controllers/port/status.go index 9a8372569..f7e466971 100644 --- a/internal/controllers/port/status.go +++ b/internal/controllers/port/status.go @@ -67,7 +67,6 @@ func (portStatusWriter) ApplyResourceStatus(log logr.Logger, osResource *osResou WithNetworkID(osResource.NetworkID). WithTags(osResource.Tags...). WithSecurityGroups(osResource.SecurityGroups...). - WithPropagateUplinkStatus(osResource.PropagateUplinkStatus). WithVNICType(osResource.VNICType). WithPortSecurityEnabled(osResource.PortSecurityEnabled). WithRevisionNumber(int64(osResource.RevisionNumber)). @@ -108,5 +107,9 @@ func (portStatusWriter) ApplyResourceStatus(log logr.Logger, osResource *osResou resourceStatus.WithTrustedVIF(*osResource.PortTrustedVIF) } + if osResource.PropagateUplinkStatusPtr != nil { + resourceStatus.WithPropagateUplinkStatus(*osResource.PropagateUplinkStatusPtr) + } + statusApply.WithResource(resourceStatus) } diff --git a/internal/controllers/port/tests/port-create-full/00-create-resource.yaml b/internal/controllers/port/tests/port-create-full/00-create-resource.yaml index 26e1d9d27..53ca4413b 100644 --- a/internal/controllers/port/tests/port-create-full/00-create-resource.yaml +++ b/internal/controllers/port/tests/port-create-full/00-create-resource.yaml @@ -87,3 +87,4 @@ spec: macAddress: fa:16:3e:23:fd:d7 hostID: id: devstack + propagateUplinkStatus: false diff --git a/internal/controllers/port/tests/port-create-minimal/00-assert.yaml b/internal/controllers/port/tests/port-create-minimal/00-assert.yaml index 9c6d861fc..ba68844fe 100644 --- a/internal/controllers/port/tests/port-create-minimal/00-assert.yaml +++ b/internal/controllers/port/tests/port-create-minimal/00-assert.yaml @@ -8,7 +8,7 @@ status: name: port-create-minimal adminStateUp: true portSecurityEnabled: true - propagateUplinkStatus: false + propagateUplinkStatus: true revisionNumber: 1 status: DOWN vnicType: normal diff --git a/internal/controllers/port/tests/port-create-sriov/00-assert.yaml b/internal/controllers/port/tests/port-create-sriov/00-assert.yaml index 3dc6e78be..f5f0205c8 100644 --- a/internal/controllers/port/tests/port-create-sriov/00-assert.yaml +++ b/internal/controllers/port/tests/port-create-sriov/00-assert.yaml @@ -9,7 +9,7 @@ status: description: Port from "create sriov" test adminStateUp: true portSecurityEnabled: false - propagateUplinkStatus: false + propagateUplinkStatus: true status: DOWN vnicType: direct tags: diff --git a/internal/controllers/port/tests/port-update/00-assert.yaml b/internal/controllers/port/tests/port-update/00-assert.yaml index 4987674c3..69cdc99e8 100644 --- a/internal/controllers/port/tests/port-update/00-assert.yaml +++ b/internal/controllers/port/tests/port-update/00-assert.yaml @@ -23,7 +23,7 @@ status: name: port-update adminStateUp: true portSecurityEnabled: false - propagateUplinkStatus: false + propagateUplinkStatus: true revisionNumber: 1 status: DOWN vnicType: normal diff --git a/internal/controllers/port/tests/port-update/01-assert.yaml b/internal/controllers/port/tests/port-update/01-assert.yaml index ee4510e2c..0fa512121 100644 --- a/internal/controllers/port/tests/port-update/01-assert.yaml +++ b/internal/controllers/port/tests/port-update/01-assert.yaml @@ -34,7 +34,7 @@ status: description: port-update-updated adminStateUp: true portSecurityEnabled: true - propagateUplinkStatus: false + propagateUplinkStatus: true status: DOWN vnicType: direct allowedAddressPairs: diff --git a/internal/controllers/port/tests/port-update/02-assert.yaml b/internal/controllers/port/tests/port-update/02-assert.yaml index a3e211d75..a96e5d05a 100644 --- a/internal/controllers/port/tests/port-update/02-assert.yaml +++ b/internal/controllers/port/tests/port-update/02-assert.yaml @@ -24,7 +24,7 @@ status: name: port-update adminStateUp: true portSecurityEnabled: false - propagateUplinkStatus: false + propagateUplinkStatus: true status: DOWN vnicType: normal conditions: diff --git a/internal/osclients/networking.go b/internal/osclients/networking.go index 088c9799c..a628d9146 100644 --- a/internal/osclients/networking.go +++ b/internal/osclients/networking.go @@ -18,6 +18,7 @@ package osclients import ( "context" + "encoding/json" "fmt" "iter" @@ -58,6 +59,41 @@ type PortExt struct { portsecurity.PortSecurityExt portsbinding.PortsBindingExt portstrustedvif.PortTrustedVIFExt + + // PropagateUplinkStatusPtr is a pointer variant of ports.Port.PropagateUplinkStatus. + // The embedded field is a non-pointer bool that defaults to false, making it + // impossible to distinguish "extension not enabled" from "explicitly false". + // This pointer allows detecting whether the field was present in the API response. + // It won't be needed with Gophercloud v3. + PropagateUplinkStatusPtr *bool `json:"propagate_uplink_status,omitempty"` +} + +// TODO(winiciusallan): Drop this custom unmarshaler once Gophercloud is +// on V3, so we have the following change +// https://github.com/gophercloud/gophercloud/pull/3609. +func (p *PortExt) UnmarshalJSON(b []byte) error { + if err := json.Unmarshal(b, &p.Port); err != nil { + return err + } + if err := json.Unmarshal(b, &p.PortSecurityExt); err != nil { + return err + } + if err := json.Unmarshal(b, &p.PortsBindingExt); err != nil { + return err + } + if err := json.Unmarshal(b, &p.PortTrustedVIFExt); err != nil { + return err + } + + var tmp struct { + PropagateUplinkStatusPtr *bool `json:"propagate_uplink_status"` + } + if err := json.Unmarshal(b, &tmp); err != nil { + return err + } + + p.PropagateUplinkStatusPtr = tmp.PropagateUplinkStatusPtr + return nil } type NetworkClient interface { @@ -189,6 +225,7 @@ func (c networkClient) ListPort(ctx context.Context, opts ports.ListOptsBuilder) } return resources, nil } + pager := ports.List(c.serviceClient, opts) return func(yield func(*PortExt, error) bool) { _ = pager.EachPage(ctx, yieldPage(extractPortExt, yield)) diff --git a/pkg/clients/applyconfiguration/api/v1alpha1/portresourcespec.go b/pkg/clients/applyconfiguration/api/v1alpha1/portresourcespec.go index 4bdf68bdb..e0935b42c 100644 --- a/pkg/clients/applyconfiguration/api/v1alpha1/portresourcespec.go +++ b/pkg/clients/applyconfiguration/api/v1alpha1/portresourcespec.go @@ -25,21 +25,22 @@ import ( // PortResourceSpecApplyConfiguration represents a declarative configuration of the PortResourceSpec type for use // with apply. type PortResourceSpecApplyConfiguration struct { - Name *apiv1alpha1.OpenStackName `json:"name,omitempty"` - Description *apiv1alpha1.NeutronDescription `json:"description,omitempty"` - NetworkRef *apiv1alpha1.KubernetesNameRef `json:"networkRef,omitempty"` - Tags []apiv1alpha1.NeutronTag `json:"tags,omitempty"` - AllowedAddressPairs []AllowedAddressPairApplyConfiguration `json:"allowedAddressPairs,omitempty"` - Addresses []AddressApplyConfiguration `json:"addresses,omitempty"` - AdminStateUp *bool `json:"adminStateUp,omitempty"` - SecurityGroupRefs []apiv1alpha1.KubernetesNameRef `json:"securityGroupRefs,omitempty"` - VNICType *string `json:"vnicType,omitempty"` - PortSecurity *apiv1alpha1.PortSecurityState `json:"portSecurity,omitempty"` - ProjectRef *apiv1alpha1.KubernetesNameRef `json:"projectRef,omitempty"` - MACAddress *string `json:"macAddress,omitempty"` - HostID *HostIDApplyConfiguration `json:"hostID,omitempty"` - TrustedVIF *bool `json:"trustedVIF,omitempty"` - ValueSpecs []PortValueSpecApplyConfiguration `json:"valueSpecs,omitempty"` + Name *apiv1alpha1.OpenStackName `json:"name,omitempty"` + Description *apiv1alpha1.NeutronDescription `json:"description,omitempty"` + NetworkRef *apiv1alpha1.KubernetesNameRef `json:"networkRef,omitempty"` + Tags []apiv1alpha1.NeutronTag `json:"tags,omitempty"` + AllowedAddressPairs []AllowedAddressPairApplyConfiguration `json:"allowedAddressPairs,omitempty"` + Addresses []AddressApplyConfiguration `json:"addresses,omitempty"` + AdminStateUp *bool `json:"adminStateUp,omitempty"` + SecurityGroupRefs []apiv1alpha1.KubernetesNameRef `json:"securityGroupRefs,omitempty"` + VNICType *string `json:"vnicType,omitempty"` + PortSecurity *apiv1alpha1.PortSecurityState `json:"portSecurity,omitempty"` + ProjectRef *apiv1alpha1.KubernetesNameRef `json:"projectRef,omitempty"` + MACAddress *string `json:"macAddress,omitempty"` + HostID *HostIDApplyConfiguration `json:"hostID,omitempty"` + TrustedVIF *bool `json:"trustedVIF,omitempty"` + ValueSpecs []PortValueSpecApplyConfiguration `json:"valueSpecs,omitempty"` + PropagateUplinkStatus *bool `json:"propagateUplinkStatus,omitempty"` } // PortResourceSpecApplyConfiguration constructs a declarative configuration of the PortResourceSpec type for use with @@ -186,3 +187,11 @@ func (b *PortResourceSpecApplyConfiguration) WithValueSpecs(values ...*PortValue } return b } + +// WithPropagateUplinkStatus sets the PropagateUplinkStatus field in the declarative configuration to the given value +// and returns the receiver, so that objects can be built by chaining "With" function invocations. +// If called multiple times, the PropagateUplinkStatus field is set to the value of the last call. +func (b *PortResourceSpecApplyConfiguration) WithPropagateUplinkStatus(value bool) *PortResourceSpecApplyConfiguration { + b.PropagateUplinkStatus = &value + return b +} diff --git a/pkg/clients/applyconfiguration/internal/internal.go b/pkg/clients/applyconfiguration/internal/internal.go index abfc36fe8..6ce4d211f 100644 --- a/pkg/clients/applyconfiguration/internal/internal.go +++ b/pkg/clients/applyconfiguration/internal/internal.go @@ -1804,6 +1804,9 @@ var schemaYAML = typed.YAMLObject(`types: - name: projectRef type: scalar: string + - name: propagateUplinkStatus + type: + scalar: boolean - name: securityGroupRefs type: list: diff --git a/website/docs/crd-reference.md b/website/docs/crd-reference.md index 1ccbb7352..c16b334ae 100644 --- a/website/docs/crd-reference.md +++ b/website/docs/crd-reference.md @@ -2785,6 +2785,7 @@ _Appears in:_ | `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
Optional: \{\}
| | `trustedVIF` _boolean_ | trustedVIF indicates whether the VF for the port will become
trusted by physical function to perform some privileged
operations. Only admin users can create ports with this field. | | Optional: \{\}
| | `valueSpecs` _[PortValueSpec](#portvaluespec) array_ | valueSpecs are extra parameters to include in the API request
with OpenStack. This is an extension point for the API, so what
they do and if they are supported, depends on the specific
OpenStack implementation. This was meant to work similar to the
property on Heat port resource. Since this depends on the
underlying implementation, we can't predict its fields, and
therefore, we don't know how to reconcile them in advance. Use
this field wisely and be aware of the expected behavior. | | MaxItems: 128
Optional: \{\}
| +| `propagateUplinkStatus` _boolean_ | propagateUplinkStatus represents the uplink status propagation of
the port.
The field is now immutable due to a limitation on
Dalmatian (2024.2) release, we should address this later.
https://github.com/k-orc/openstack-resource-controller/pull/641#discussion_r2694783787 | | Optional: \{\}
| #### PortResourceStatus