Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions api/v1alpha1/port_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ type PortResourceSpec struct {
// +kubebuilder:validation:MaxLength=32
// +optional
MACAddress string `json:"macAddress,omitempty"`

// hostID is the ID of host where the port resides.
// +kubebuilder:validation:MaxLength=36
// +optional
HostID string `json:"hostID,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wished there was a way to represent a Host resource in ORC so that we could have a HostRef rather than a HostID.
In that case, HostID is an ID that is exposed to a user via the nova API in the server object and that is hashed from the project ID.

According to the nova docs:

An ID string representing the host. This is a hashed value so will not actually look like a hostname, and is hashed with data from the project_id, so the same physical host as seen by two different project_ids, will be different. It is useful when within the same project you need to determine if two instances are on the same or different physical hosts for the purposes of availability or performance.

I don't think there's a way to list all host IDs for a given project as a user.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a HostRef instead of simply the HostID. The only thing that we have is os-hypervisor but it is not project-scoped, so we can't the get the hashed value directly from the API =/.

}

type PortResourceStatus struct {
Expand Down Expand Up @@ -272,6 +277,11 @@ type PortResourceStatus struct {
// +optional
PortSecurityEnabled *bool `json:"portSecurityEnabled,omitempty"`

// hostID is the ID of host where the port resides.
// +kubebuilder:validation:MaxLength=128
// +optional
HostID string `json:"hostID,omitempty"`

NeutronStatusMetadata `json:",inline"`
}

Expand Down
14 changes: 14 additions & 0 deletions cmd/models-schema/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions config/crd/bases/openstack.k-orc.cloud_ports.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,10 @@ spec:
maxLength: 255
minLength: 1
type: string
hostID:
description: hostID is the ID of host where the port resides.
maxLength: 36
type: string
macAddress:
description: macAddress is the MAC address of the port.
maxLength: 32
Expand Down Expand Up @@ -561,6 +565,10 @@ spec:
maxItems: 128
type: array
x-kubernetes-list-type: atomic
hostID:
description: hostID is the ID of host where the port resides.
maxLength: 128
type: string
macAddress:
description: macAddress is the MAC address of the port.
maxLength: 1024
Expand Down
11 changes: 11 additions & 0 deletions internal/controllers/port/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ func (actuator portActuator) CreateResource(ctx context.Context, obj *orcv1alpha
portsBindingOpts := portsbinding.CreateOptsExt{
CreateOptsBuilder: createOpts,
VNICType: resource.VNICType,
HostID: resource.HostID,
}

portSecurityOpts := portsecurity.PortCreateOptsExt{
Expand Down Expand Up @@ -504,6 +505,16 @@ func handlePortBindingUpdate(updateOpts ports.UpdateOptsBuilder, resource *resou
}
}
}

if resource.HostID != "" {
if resource.HostID != osResource.HostID {
updateOpts = &portsbinding.UpdateOptsExt{
UpdateOptsBuilder: updateOpts,
HostID: &resource.HostID,
}
}
}

return updateOpts
}

Expand Down
3 changes: 2 additions & 1 deletion internal/controllers/port/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ func (portStatusWriter) ApplyResourceStatus(log logr.Logger, osResource *osResou
WithPortSecurityEnabled(osResource.PortSecurityEnabled).
WithRevisionNumber(int64(osResource.RevisionNumber)).
WithCreatedAt(metav1.NewTime(osResource.CreatedAt)).
WithUpdatedAt(metav1.NewTime(osResource.UpdatedAt))
WithUpdatedAt(metav1.NewTime(osResource.UpdatedAt)).
WithHostID(osResource.HostID)

if osResource.Description != "" {
resourceStatus.WithDescription(osResource.Description)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ status:
portSecurityEnabled: true
propagateUplinkStatus: false
status: DOWN
vnicType: direct
vnicType: macvtap
macAddress: fa:16:3e:23:fd:d7
hostID: devstack
tags:
- tag1
---
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ spec:
- subnetRef: port-create-full
ip: 192.168.155.122
portSecurity: Enabled
vnicType: direct
vnicType: macvtap
projectRef: port-create-full
macAddress: fa:16:3e:23:fd:d7
hostID: devstack
30 changes: 30 additions & 0 deletions internal/controllers/port/tests/port-update/00-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,20 @@ 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 != ''"
- celExpr: "port.status.resource.updatedAt != ''"
- 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 == ''"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny, I would have expected the binding host_id to be returned always to the admin, but according to what you're saying (and the tests) it's only shown when a binding host_id was set in the request?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's right. I made a few tests using curl to avoid meta commands hiding the output, and the binding:host_id was only returned when set in the request.

I believe it is because we explicitly specify which host we want to bind to that port. On the other hand, I think it will be defined when binding to a device (e.g., server or router), or via Port Binding, like we are doing here.

Passing hostID:

❯ curl -s -X GET -H "X-Auth-Token: $TOKEN" http://10.5.11.133/networking/v2.0/ports/$(k get port port-create-minimal -ojsonpath='{.status.id}') | jq .
{
   "port":{
      "mac_address":"fa:16:3e:f4:f8:3c",
      "admin_state_up":true,
      "status":"DOWN",
      "binding:vnic_type":"normal",
      "binding:profile":{},
      "binding:host_id":"devstack",
      "binding:vif_type":"ovs",
      "binding:vif_details":{
         "port_filter":true,
         "connectivity":"l2",
         "bridge_name":"br-int",
         "datapath_type":"system",
         "bound_drivers":{
            "0":"ovn"
         }
      }
   }
}

Not passing hostID:

{
  "port": {
    "mac_address": "fa:16:3e:44:a0:7d",
    "admin_state_up": true,
    "status": "DOWN",
    "binding:vnic_type": "normal",
    "binding:profile": {},
    "binding:host_id": "",
    "binding:vif_type": "unbound",
    "binding:vif_details": {},
    "port_security_enabled": true,
    "trusted": null,
  }
}

---
apiVersion: openstack.k-orc.cloud/v1alpha1
kind: Port
Expand All @@ -36,3 +43,26 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,17 @@ 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
17 changes: 16 additions & 1 deletion internal/controllers/port/tests/port-update/01-assert.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,19 @@ status:
reason: Success
- type: Progressing
status: "False"
reason: Success
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,16 @@ spec:
tags:
- tag1
vnicType: direct
portSecurity: Enabled
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: devstack
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ apiVersion: kuttl.dev/v1beta1
kind: TestStep
commands:
- command: kubectl replace -f 00-minimal-resource.yaml
namespaced: true
namespaced: true

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions pkg/clients/applyconfiguration/internal/internal.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions website/docs/crd-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -2171,6 +2171,7 @@ _Appears in:_
| `portSecurity` _[PortSecurityState](#portsecuritystate)_ | portSecurity controls port security for this port.<br />When set to Enabled, port security is enabled.<br />When set to Disabled, port security is disabled and SecurityGroupRefs must be empty.<br />When set to Inherit (default), it takes the value from the network level. | Inherit | Enum: [Enabled Disabled Inherit] <br /> |
| `projectRef` _[KubernetesNameRef](#kubernetesnameref)_ | projectRef is a reference to the ORC Project this resource is associated with.<br />Typically, only used by admin. | | MaxLength: 253 <br />MinLength: 1 <br /> |
| `macAddress` _string_ | macAddress is the MAC address of the port. | | MaxLength: 32 <br /> |
| `hostID` _string_ | hostID is the ID of host where the port resides. | | MaxLength: 36 <br /> |


#### PortResourceStatus
Expand Down Expand Up @@ -2202,6 +2203,7 @@ _Appears in:_
| `propagateUplinkStatus` _boolean_ | propagateUplinkStatus represents the uplink status propagation of<br />the port. | | |
| `vnicType` _string_ | vnicType is the type of vNIC which this port is attached to. | | MaxLength: 64 <br /> |
| `portSecurityEnabled` _boolean_ | portSecurityEnabled indicates whether port security is enabled or not. | | |
| `hostID` _string_ | hostID is the ID of host where the port resides. | | MaxLength: 128 <br /> |
| `createdAt` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#time-v1-meta)_ | createdAt shows the date and time when the resource was created. The date and time stamp format is ISO 8601 | | |
| `updatedAt` _[Time](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.29/#time-v1-meta)_ | updatedAt shows the date and time when the resource was updated. The date and time stamp format is ISO 8601 | | |
| `revisionNumber` _integer_ | revisionNumber optionally set via extensions/standard-attr-revisions | | |
Expand Down