Skip to content
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

Jump to bottom

feat: Add ability to specify MTU when creating Network #1262

Merged
merged 5 commits into from
Jul 2, 2024
Merged

feat: Add ability to specify MTU when creating Network #1262

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
857b045
feat: Allow ability to set mtu in infrav1.NetworkSpec
tasdikrahman Jun 14, 2024
78a8390
feat: Allow Setting the desired GCPCluster.Spec.Network.Mtu passed wh…
tasdikrahman Jun 14, 2024
d265963
feat: Add minimum value for MTU to be 1300 and maximum to be 8896
tasdikrahman Jul 1, 2024
b0aaa8f
chore: GCPCluster webhook: Throw error if MTU in the new incoming spe…
tasdikrahman Jul 2, 2024
e3a61fc
chore: NetworkSpec: Add kubebuilder default of 1460 for MTU if not sp…
tasdikrahman Jul 2, 2024
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
6 changes: 6 additions & 0 deletions api/v1beta1/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ type NetworkSpec struct {
// HostProject is the name of the project hosting the shared VPC network resources.
// +optional
HostProject *string `json:"hostProject,omitempty"`

// Mtu: Maximum Transmission Unit (MTU), in bytes, of packets passing through
// this interconnect attachment. Only 1440 and 1500 are allowed. If not
// specified, the value will default to 1440.
// +optional
Mtu int64 `json:"mtu,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

I think we should add openAPI validation for this field only allowing the two values you are mentioning.
Also where are those values coming from? We might want to leave a message mentioning that.

Copy link
Member

Choose a reason for hiding this comment

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

Also we could add a validation step in the validation code (e.g. webooks).
And a couple of unit test checking that is validated correctly.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should add openAPI validation for this field only allowing the two values you are mentioning.
Also where are those values coming from? We might want to leave a message mentioning that.

Hello there, I fixed the validation code to have the minimum and maximum values present. The values of minimum of 1300 and maximum of 8896 are taken from https://pkg.go.dev/google.golang.org/api/compute/v1#Network

Also we could add a validation step in the validation code (e.g. webooks).
And a couple of unit test checking that is validated correctly.

Will add some unit tests exercising this.

}

// LoadBalancerType defines the Load Balancer that should be created.
Expand Down
10 changes: 10 additions & 0 deletions cloud/scope/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,15 @@ func (s *ClusterScope) NetworkName() string {
return ptr.Deref(s.GCPCluster.Spec.Network.Name, "default")
}

// NetworkMtu returns the Network MTU of 1440 which is the default, otherwise returns back what is being set.
// https://cloud.google.com/vpc/docs/mtu
func (s *ClusterScope) NetworkMtu() int64 {
if s.GCPCluster.Spec.Network.Mtu == 0 {
return int64(1440)
}
return s.GCPCluster.Spec.Network.Mtu
}

// NetworkLink returns the partial URL for the network.
func (s *ClusterScope) NetworkLink() string {
return fmt.Sprintf("projects/%s/global/networks/%s", s.NetworkProject(), s.NetworkName())
Expand Down Expand Up @@ -206,6 +215,7 @@ func (s *ClusterScope) NetworkSpec() *compute.Network {
Description: infrav1.ClusterTagKey(s.Name()),
AutoCreateSubnetworks: createSubnet,
ForceSendFields: []string{"AutoCreateSubnetworks"},
Mtu: s.NetworkMtu(),
}

return network
Expand Down
7 changes: 7 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclusters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ spec:
(useful for changing apiserver port)
format: int32
type: integer
mtu:
description: |-
Mtu: Maximum Transmission Unit (MTU), in bytes, of packets passing through
this interconnect attachment. Only 1440 and 1500 are allowed. If not
specified, the value will default to 1440.
format: int64
type: integer
name:
description: Name is the name of the network to be used.
type: string
Expand Down
7 changes: 7 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_gcpclustertemplates.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,13 @@ spec:
backend (useful for changing apiserver port)
format: int32
type: integer
mtu:
description: |-
Mtu: Maximum Transmission Unit (MTU), in bytes, of packets passing through
this interconnect attachment. Only 1440 and 1500 are allowed. If not
specified, the value will default to 1440.
format: int64
type: integer
name:
description: Name is the name of the network to be used.
type: string
Expand Down
7 changes: 7 additions & 0 deletions config/crd/bases/infrastructure.cluster.x-k8s.io_gcpmanagedclusters.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,13 @@ spec:
(useful for changing apiserver port)
format: int32
type: integer
mtu:
description: |-
Mtu: Maximum Transmission Unit (MTU), in bytes, of packets passing through
this interconnect attachment. Only 1440 and 1500 are allowed. If not
specified, the value will default to 1440.
format: int64
type: integer
name:
description: Name is the name of the network to be used.
type: string
Expand Down