Skip to content

Add loadbalancer field to Power VS machine provider spec #1415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
openshift-merge-robot merged 1 commit into from
Apr 5, 2023
Merged

Add loadbalancer field to Power VS machine provider spec #1415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 36 additions & 1 deletion machine/v1/types_powervsprovider.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ type PowerVSResourceType string
// PowerVSProcessorType enum attribute to identify the PowerVS instance processor type
type PowerVSProcessorType string

// IBMVPCLoadBalancerType is the type of LoadBalancer to use when registering
// an instance with load balancers specified in LoadBalancerNames
type IBMVPCLoadBalancerType string

// ApplicationLoadBalancerType is possible values for IBMVPCLoadBalancerType.
const (
ApplicationLoadBalancerType IBMVPCLoadBalancerType = "Application" // Application Load Balancer for VPC (ALB)
)

const (
// PowerVSResourceTypeID enum property to identify an ID type resource reference
PowerVSResourceTypeID PowerVSResourceType = "ID"
Expand Down Expand Up @@ -121,6 +130,11 @@ type PowerVSMachineProviderConfig struct {
// default, which is subject to change over time. The current default is 32.
// +optional
MemoryGiB int32 `json:"memoryGiB,omitempty"`

// loadBalancers is the set of load balancers to which the new control plane instance
// should be added once it is created.
// +optional
LoadBalancers []LoadBalancerReference `json:"loadBalancers,omitempty"`
}

// PowerVSResource is a reference to a specific PowerVS resource by ID, Name or RegEx
Expand Down Expand Up @@ -150,7 +164,7 @@ type PowerVSResource struct {
//
// Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer).
// +openshift:compatibility-gen:level=1
//+k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
type PowerVSMachineProviderStatus struct {
metav1.TypeMeta `json:",inline"`

Expand Down Expand Up @@ -190,3 +204,24 @@ type PowerVSSecretReference struct {
// +optional
Name string `json:"name,omitempty"`
}

// LoadBalancerReference is a reference to a load balancer on IBM Cloud virtual private cloud(VPC).
type LoadBalancerReference struct {
// name of the LoadBalancer in IBM Cloud VPC.
// The name should be between 1 and 63 characters long and may consist of lowercase alphanumeric characters and hyphens only.
// The value must not end with a hyphen.
// It is a reference to existing LoadBalancer created by openshift installer component.
// +kubebuilder:validation:Required
// +kubebuilder:validation:Pattern=`^([a-z]|[a-z][-a-z0-9]*[a-z0-9]|[0-9][-a-z0-9]*([a-z]|[-a-z][-a-z0-9]*[a-z0-9]))$`
// +kubebuilder:validation:MinLength=1
// +kubebuilder:validation:MaxLength=63
Name string `json:"name"`
Copy link
Contributor

@JoelSpeed JoelSpeed Mar 10, 2023

Choose a reason for hiding this comment

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

Lets add a kubebuilder validation for the alpha numeric characters and hyphen, then we will want to explain this limitation within the godoc, and also, we should probably set a maximum length, is there anything in IBM that limits the length or the characters?

Copy link
Contributor

@JoelSpeed JoelSpeed Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I found the following in the docs, how wil we handle this? Convert any hyphen to an underscore internally? How will we limit the name length given the infra-id may be 27 characters, any thoughts?

Names may not be empty. A valid load balancer name starts with a letter, followed by letters, digits, or underscores. The length of the name may not exceed 40 characters.

Copy link
Member Author

@Karthik-K-N Karthik-K-N Mar 10, 2023

Choose a reason for hiding this comment

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

Loadbalancer name may contain hyphens, Currently this is how the installer creates the loadbalancer rdr-kn08031125-vhwtg-loadbalancer.
From api doc I can see this description for name

The globally unique name for this load balancer profile

Possible values: 1 ≤ length ≤ 63, Value must match regular expression ^([a-z]|[a-z][-a-z0-9]*[a-z0-9]|[0-9][-a-z0-9]*([a-z]|[-a-z][-a-z0-9]*[a-z0-9]))$

Example: network-fixed

Copy link
Contributor

@JoelSpeed JoelSpeed Apr 5, 2023

Choose a reason for hiding this comment

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

Great, thanks for finding that, can you explain this in prose within the godoc please? We need to say something along the lines of

// The name should be between 1 and 63 characters long and may consist of lowercase alphanumeric characters and hyphens only.
// The value must not end with a hyphen and integer values are not allowed.

I think I've understood that regex correctly anyway

Copy link
Member Author

@Karthik-K-N Karthik-K-N Apr 5, 2023

Choose a reason for hiding this comment

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

If we can just add a touch more documentation to this name field as per my suggestion I think this is good to go

Updated accordingly, Please take a look. Thank you.

// type of the LoadBalancer service supported by IBM Cloud VPC.
// Currently, only Application LoadBalancer is supported.
// More details about Application LoadBalancer
// https://cloud.ibm.com/docs/vpc?topic=vpc-load-balancers-about&interface=ui
// Supported values are Application.
// +kubebuilder:validation:Required
// +kubebuilder:validation:Enum:="Application"
Type IBMVPCLoadBalancerType `json:"type"`
}
21 changes: 21 additions & 0 deletions machine/v1/zz_generated.deepcopy.go
View file
Open in desktop

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

11 changes: 11 additions & 0 deletions machine/v1/zz_generated.swagger_doc_generated.go
View file
Open in desktop

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

47 changes: 46 additions & 1 deletion openapi/generated_openapi/zz_generated.openapi.go
View file
Open in desktop

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

28 changes: 28 additions & 0 deletions openapi/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16745,6 +16745,26 @@
}
}
},
"com.github.openshift.api.machine.v1.LoadBalancerReference": {
"description": "LoadBalancerReference is a reference to a load balancer on IBM Cloud virtual private cloud(VPC).",
"type": "object",
"required": [
"name",
"type"
],
"properties": {
"name": {
"description": "name of the LoadBalancer in IBM Cloud VPC. The name should be between 1 and 63 characters long and may consist of lowercase alphanumeric characters and hyphens only. The value must not end with a hyphen. It is a reference to existing LoadBalancer created by openshift installer component.",
"type": "string",
"default": ""
},
"type": {
"description": "type of the LoadBalancer service supported by IBM Cloud VPC. Currently, only Application LoadBalancer is supported. More details about Application LoadBalancer https://cloud.ibm.com/docs/vpc?topic=vpc-load-balancers-about&interface=ui Supported values are Application.",
"type": "string",
"default": ""
}
}
},
"com.github.openshift.api.machine.v1.NutanixCategory": {
"description": "NutanixCategory identifies a pair of prism category key and value",
"type": "object",
Expand Down Expand Up @@ -16977,6 +16997,14 @@
"description": "Kind is a string value representing the REST resource this object represents. Servers may infer this from the endpoint the client submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds",
"type": "string"
},
"loadBalancers": {
"description": "loadBalancers is the set of load balancers to which the new control plane instance should be added once it is created.",
"type": "array",
"items": {
"default": {},
"$ref": "#/definitions/com.github.openshift.api.machine.v1.LoadBalancerReference"
}
},
"memoryGiB": {
"description": "memoryGiB is the size of a virtual machine's memory, in GiB. maximum value for the MemoryGiB depends on the selected SystemType. when SystemType is set to e880 maximum MemoryGiB value is 7463 GiB. when SystemType is set to e980 maximum MemoryGiB value is 15307 GiB. when SystemType is set to s922 maximum MemoryGiB value is 942 GiB. The minimum memory is 32 GiB. When omitted, this means the user has no opinion and the platform is left to choose a reasonable default, which is subject to change over time. The current default is 32.",
"type": "integer",
Expand Down