feat(): layer2 support for CAPP #787

rahulii · 2024-08-22T09:36:06Z

✨ API and CRD changes for L2 support

This draft PR implements the API changes for design described in design doc (L2 design). This branch is being used to work through the design, build intuition, and gain early insights and feedback.

What this PR does / why we need it:

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #613

Signed-off-by: rahulii <[email protected]>

k8s-ci-robot · 2024-08-22T09:36:14Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rahulii
Once this PR has been reviewed and has the lgtm label, please assign vincepri for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Signed-off-by: rahulii <[email protected]>

niravparikh05

lgtm

Signed-off-by: rahulii <[email protected]>

displague · 2024-08-29T15:21:21Z

This draft PR implements the API changes for design described in #786 (L2 design). This branch is being used to work through the design, build intuition, and gain early insights and feedback.

Converting to draft. This can't be merged until the controllers can meaningfully reconcile the proposed CRD changes and provision Equinix Metal devices to fit the desired spec.

Signed-off-by: rahulii <[email protected]>

nirav-rafay

few minor comments

nirav-rafay · 2024-09-26T13:40:48Z

api/v1beta1/packetmachine_types.go

+ // If not specified, the first available IP address from the IP address range will be assigned.
+ // This is useful when you want to reserve some IP addresses for other purposes for eg Gateways, DNS etc.
+ // +optional
+ AssignmentRange string `json:"assignmentRange,omitempty"`


can you specify the expected format here ? Also add a validation if the format in valid.

Also can a user configure multiple address ranges as below ?
e.g. 10.60.10.2-10.60.10.9, 10.60.10.11-10.60.10.20

nirav-rafay · 2024-09-26T13:50:06Z

internal/layer2/layer2.go

+}
+
+func (c *Config) GetTemplate() (string, error) {
+ tmpl, err := template.New("layer-2-user-data").Parse(configTemplate)


probably mode this under NewConfig(), we need to initialize and parse the template only once.

And move the tmpl.Execute as a separate utility method.

nirav-rafay · 2024-09-26T13:53:38Z

internal/layer2/mime.go

+// GenerateInitDocument generates a multipart document with cloud-config data
+func GenerateInitDocument(rawUserData, layer2UserData string) (string, error) {
+ var buf bytes.Buffer
+ mpWriter := multipart.NewWriter(&buf)


Suggested change

mpWriter := multipart.NewWriter(&buf)

mpWriter := multipart.NewWriter(&buf)

defer mpWriter.Close()

internal/layer2/template.go

nirav-rafay · 2024-09-26T14:16:46Z

internal/layer2/layer2.go

+
+ // execute the template and save the output to a buffer
+ var output bytes.Buffer
+ if err := tmpl.Execute(&output, c); err != nil {


support for adding route configuration to user data script

niravparikh05 · 2024-09-26T14:33:37Z

internal/layer2/mime.go

+)
+
+// GenerateInitDocument generates a multipart document with cloud-config data
+func GenerateInitDocument(rawUserData, layer2UserData string) (string, error) {


let's make this more generic, something like append input user data and multiple clould config could be added as deemed necessary

niravparikh05 · 2024-09-26T14:45:13Z

@rahuli few noted todo's

Move the layer2 user data template to the default flavor PacketMachineTemplate spec found in templates/cluster-template.yaml
Opportunity to use generic ipam provider instead of writing it on our own

Signed-off-by: rahulii <[email protected]>

nirav-rafay

@rahulii looks fine overall, few comments on making this approach more generic.

nirav-rafay · 2024-10-08T08:46:47Z

config/default/manager_image_patch.yaml

@@ -8,5 +8,5 @@ spec:
 spec:
 containers:
 # Change the value of image field below to your controller image URL
- - image: quay.io/equinix-oss/cluster-api-provider-packet:dev
+ - image: docker.io/rahulsawra/cluster-api-provider-packet-amd64:dev


keep this for your local testing only

nirav-rafay · 2024-10-08T08:47:14Z

config/default/manager_pull_policy.yaml

@@ -8,4 +8,4 @@ spec:
 spec:
 containers:
 - name: manager
- imagePullPolicy: IfNotPresent
+ imagePullPolicy: Always


local testing only

nirav-rafay · 2024-10-08T09:03:47Z

internal/layer2/merge.go

+ if bootstrapStripped != "" {
+ if err := yaml.Unmarshal([]byte(bootstrapStripped), &bootstrapConfig); err != nil {
+ return "", fmt.Errorf("error parsing bootstrap YAML: %v", err)
+ }
+ } else {
+ bootstrapConfig = make(map[string]interface{})
+ }
+
+ if layer2Stripped != "" {
+ if err := yaml.Unmarshal([]byte(layer2Stripped), &layer2UserDataConfig); err != nil {
+ return "", fmt.Errorf("error parsing layer2 YAML: %v", err)
+ }
+ } else {
+ layer2UserDataConfig = make(map[string]interface{})
+ }


bootstrapConfig := make(map[string]interface{}) if err := yaml.Unmarshal([]byte(layer2Stripped), &layer2UserDataConfig); err != nil { return "", fmt.Errorf("error parsing layer2 YAML: %v", err) } layer2UserDataConfig := make(map[string]interface{}) if err := yaml.Unmarshal([]byte(layer2Stripped), &layer2UserDataConfig); err != nil { return "", fmt.Errorf("error parsing layer2 YAML: %v", err) }

nirav-rafay · 2024-10-08T09:07:49Z

internal/layer2/merge.go

+
+ for i, line := range lines {
+ trimmedLine := strings.TrimSpace(line)
+ switch trimmedLine {


this assumes there will not be any other header !

Can we modify this to be more generic - is there a way to identify headers apart from string equals

nirav-rafay · 2024-10-08T09:10:47Z

internal/layer2/merge.go

+}
+
+// MergeConfigs combines bootstrap data with layer2 config
+func (m *CloudConfigMerger) MergeConfigs(bootstrapData string, layer2UserData string) (string, error) {


can we make this generic as well ?

CloudConfigMerger.WithBootstrapData(bdata). WithLayer2Data(l2data). WithAddlUserDatas([]userdatas). // this can be optional and can be extensible MergeConfigs()

Signed-off-by: rahulii <[email protected]>

ipam support

linux-foundation-easycla · 2024-10-11T12:28:32Z

The committers listed above are authorized under a signed CLA.

✅ login: rahulii / name: Rahul Sawra (ebfddac, 4cfb7b8, 05cbc0c, 1d2b71e, 15aaba7, f762e5b, 16fdf8d, e1a19b5, cdf7155, ddf37d9, d901fe6, a452289, 25510bd, fcfcaac, d4d4b25, b7e1120, 4f9c8fc, 4c64c5d, c55ea3f, 6207973, 07516df, 4efbfd8, b7e871a)
✅ login: vardhaman-surana / name: Vardhaman Surana (5f33a86, aad4223, 4d30dcc)

Signed-off-by: rahulii <[email protected]>

controllers/packetmachine_controller.go

nirav-rafay · 2024-10-14T06:21:36Z

controllers/packetmachine_controller.go

+// assignVXLAN assigns a VXLAN to a port
+func (r *PacketMachineReconciler) assignVXLAN(ctx context.Context, portID, vxlanStr string) error {
+ _, resp, err := r.PacketClient.PortsApi.AssignPort(ctx, portID).PortAssignInput(metal.PortAssignInput{
+ Vnid: &vxlanStr,


let's check if this is anyway to fetch vxlan id from vlan value - for ease of configuration / usability

There are examples here: https://github.com/equinix/terraform-provider-equinix/blob/343c8fc6a8d3674f3067629e146855c1d3d96dbe/internal/resources/metal/port/helpers.go#L145
Either vlan_ids or vxlan_ids are accepted in the Terraform equinix_metal_port resource. The batch port assignment API accepts either format.

https://deploy.equinix.com/developers/api/metal/#tag/Ports/operation/createPortVlanAssignmentBatch

Vnid in the PortAssignInput will also take either format: https://deploy.equinix.com/developers/api/metal/#tag/Ports/operation/assignPort

In Terraform, we had to be careful in some usage so that Terraform wouldn't get into a drift-detected state in places where an Equinix Metal API VLAN ID (UUID) could be returned in response despite a VXLAN ID (1234) value being supplied in the request.

Signed-off-by: rahulii <[email protected]>

added ipaddress claim deletion

Signed-off-by: rahulii <[email protected]>

API and CRD changes for adding layer2 support

05cbc0c

Signed-off-by: rahulii <[email protected]>

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Aug 22, 2024

k8s-ci-robot requested review from cprivitere and displague August 22, 2024 09:36

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 22, 2024

json tag fix

15aaba7

Signed-off-by: rahulii <[email protected]>

rahulii marked this pull request as ready for review August 27, 2024 12:48

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 27, 2024

k8s-ci-robot requested a review from detiber August 27, 2024 12:48

displague mentioned this pull request Aug 27, 2024

docs(design): phase-1 docs for layer2 support in CAPP: version 1 #786

Draft

niravparikh05 reviewed Aug 29, 2024

View reviewed changes

change ipAddressReservation to address

ebfddac

Signed-off-by: rahulii <[email protected]>

displague marked this pull request as draft August 29, 2024 15:21

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 29, 2024

rahulii added 2 commits September 17, 2024 12:27

make changes to the api as per design doc changes

1d2b71e

Signed-off-by: rahulii <[email protected]>

minor change

e1a19b5

Signed-off-by: rahulii <[email protected]>

rahulii changed the title ~~API and CRD changes for adding layer2 support~~ feat(): layer2 support for CAPP Sep 24, 2024

api changes and user-data implementation

cdf7155

Signed-off-by: rahulii <[email protected]>

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 25, 2024

add IPAddressManagement Service

4cfb7b8

Signed-off-by: rahulii <[email protected]>

nirav-rafay reviewed Sep 26, 2024

View reviewed changes

niravparikh05 reviewed Sep 26, 2024

View reviewed changes

merge bootstrap and layer2 cloud configs

f762e5b

Signed-off-by: rahulii <[email protected]>

nirav-rafay reviewed Oct 8, 2024

View reviewed changes

add unit test cases

a452289

Signed-off-by: rahulii <[email protected]>

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 8, 2024

vardhaman-surana and others added 8 commits October 8, 2024 18:01

ipam support

5f33a86

added rbac

aad4223

minor fix

25510bd

Signed-off-by: rahulii <[email protected]>

reconcile port settings for layer2 and bonded port

16fdf8d

Signed-off-by: rahulii <[email protected]>

minor fix

b7e1120

Signed-off-by: rahulii <[email protected]>

refactor code and add static routes

fcfcaac

Signed-off-by: rahulii <[email protected]>

bug fix

b7e871a

Signed-off-by: rahulii <[email protected]>

Merge pull request #1 from rahulii/ipam

ddf37d9

ipam support

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 11, 2024

delete unused code

d901fe6

Signed-off-by: rahulii <[email protected]>

nirav-rafay reviewed Oct 14, 2024

View reviewed changes

minor fix

c55ea3f

Signed-off-by: rahulii <[email protected]>

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Oct 14, 2024

rahulii and others added 7 commits October 14, 2024 13:45

minor fix

4c64c5d

Signed-off-by: rahulii <[email protected]>

change failure to failed in state

d4d4b25

Signed-off-by: rahulii <[email protected]>

added ipaddress claim deletion

4d30dcc

Merge pull request #2 from vardhaman-surana/ipaddressclaim-deletion

4f9c8fc

added ipaddress claim deletion

add logs

4efbfd8

Signed-off-by: rahulii <[email protected]>

add routes at network file and change user-data to make them persistent

07516df

Signed-off-by: rahulii <[email protected]>

fix a bug where multiple vxlans are involved

6207973

Signed-off-by: rahulii <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(): layer2 support for CAPP #787

feat(): layer2 support for CAPP #787

rahulii commented Aug 22, 2024 •

edited by displague

Loading

k8s-ci-robot commented Aug 22, 2024

niravparikh05 left a comment

displague commented Aug 29, 2024

nirav-rafay left a comment

nirav-rafay Sep 26, 2024

nirav-rafay Sep 26, 2024

nirav-rafay Sep 26, 2024

nirav-rafay Sep 26, 2024

niravparikh05 Sep 26, 2024

niravparikh05 commented Sep 26, 2024 •

edited

Loading

nirav-rafay left a comment •

edited

Loading

nirav-rafay Oct 8, 2024

nirav-rafay Oct 8, 2024

nirav-rafay Oct 8, 2024

nirav-rafay Oct 8, 2024

nirav-rafay Oct 8, 2024

linux-foundation-easycla bot commented Oct 11, 2024 •

edited

Loading

nirav-rafay Oct 14, 2024

displague Oct 16, 2024

displague Oct 16, 2024 •

edited

Loading

	mpWriter := multipart.NewWriter(&buf)
	mpWriter := multipart.NewWriter(&buf)
	defer mpWriter.Close()

feat(): layer2 support for CAPP #787

Are you sure you want to change the base?

feat(): layer2 support for CAPP #787

Conversation

rahulii commented Aug 22, 2024 • edited by displague Loading

k8s-ci-robot commented Aug 22, 2024

niravparikh05 left a comment

Choose a reason for hiding this comment

displague commented Aug 29, 2024

nirav-rafay left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niravparikh05 commented Sep 26, 2024 • edited Loading

nirav-rafay left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

displague Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

rahulii commented Aug 22, 2024 •

edited by displague

Loading

niravparikh05 commented Sep 26, 2024 •

edited

Loading

nirav-rafay left a comment •

edited

Loading

linux-foundation-easycla bot commented Oct 11, 2024 •

edited

Loading

displague Oct 16, 2024 •

edited

Loading