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.

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

fix: add UntaggedNetwork and remove NetworkTypeCustom #88

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

FrankYang0529
Copy link
Member

@FrankYang0529 FrankYang0529 commented Feb 27, 2024

issue: harvester/harvester#5226

Test

Setup: Create a Harvester cluster

Case 1: new terraform provider

  1. Build terraform-harvester-provider.
CGO_ENABLED=0 go build -mod=vendor -o bin/terraform-provider-harvester .
mkdir -p ~/.terraform.d/plugins/registry.terraform.io/harvester/harvester/0.0.0-dev/linux_amd64
cp bin/terraform-provider-harvester ~/.terraform.d/plugins/registry.terraform.io/harvester/harvester/0.0.0-dev/linux_amd64/terraform-provider-harvester_v0.0.0-dev
  1. Create a temporary folder and put the following content in versions.tf.
terraform {
  required_providers {
    harvester = {
      source  = "harvester/harvester"
      /* version = "0.6.4" */
      version = "0.0.0-dev"
    }
  }
}

provider "harvester" {
  kubeconfig = "~/.kube/config"
}
  1. Put the following content in main.tf in the temporary folder.
data "harvester_clusternetwork" "mgmt" {
  name = "mgmt"
}

resource "harvester_network" "mgmt-untagged-network" {
  name      = "mgmt-untagged-network"
  namespace = "harvester-public"

  vlan_id = 0

  route_mode           = "auto"
  route_dhcp_server_ip = ""

  cluster_network_name = data.harvester_clusternetwork.mgmt.name
}

resource "harvester_network" "mgmt-vlan1" {
  name      = "mgmt-vlan1"
  namespace = "harvester-public"

  vlan_id = 1

  route_mode           = "auto"
  route_dhcp_server_ip = ""

  cluster_network_name = data.harvester_clusternetwork.mgmt.name
}
  1. Run terraform init and terraform apply. There are two VM Networks without error.
  2. Run terraform destroy to remove all data.

Case 2: Upgrade path

  1. Create a temporary folder and put the following content in versions.tf.
terraform {
  required_providers {
    harvester = {
      source  = "harvester/harvester"
      version = "0.6.4"
      /* version = "0.0.0-dev" */
    }
  }
}

provider "harvester" {
  kubeconfig = "~/.kube/config"
}
  1. Put the following content in main.tf in the temporary folder.
data "harvester_clusternetwork" "mgmt" {
  name = "mgmt"
}

resource "harvester_network" "mgmt-vlan1" {
  name      = "mgmt-vlan1"
  namespace = "harvester-public"

  vlan_id = 1

  route_mode           = "auto"
  route_dhcp_server_ip = ""

  cluster_network_name = data.harvester_clusternetwork.mgmt.name
}
  1. Run terraform init and terraform apply. There is one VM Networks without error.
  2. Build terraform-harvester-provider.
CGO_ENABLED=0 go build -mod=vendor -o bin/terraform-provider-harvester .
mkdir -p ~/.terraform.d/plugins/registry.terraform.io/harvester/harvester/0.0.0-dev/linux_amd64
cp bin/terraform-provider-harvester ~/.terraform.d/plugins/registry.terraform.io/harvester/harvester/0.0.0-dev/linux_amd64/terraform-provider-harvester_v0.0.0-dev
  1. Update versions.tf to use 0.0.0-dev.
  2. Run terraform init --upgrade and terraform apply. No change happen.
data.harvester_clusternetwork.mgmt: Reading...
data.harvester_clusternetwork.mgmt: Read complete after 0s [id=mgmt]
harvester_network.mgmt-vlan1: Refreshing state... [id=harvester-public/mgmt-vlan1]

No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration and found no differences, so no changes are needed.

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

Copy link
Member

@starbops starbops left a comment

Choose a reason for hiding this comment

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

I'm not sure if I understand it correctly. The labels here seem to be a hackish way of passing values among different custom processors. And they will remain on the resulting NAD object. The mutator is responsible for populating those labels using the .spec.config field. I'd suggest we leverage the NetConf struct in the network controller project to process the NAD object better. We can then serialize it in the Result() function. But if I get the wrong perception about how the Terraform provider works, please ignore my 2c. Thank you.

internal/provider/network/resource_network_constructor.go Outdated Show resolved Hide resolved
Copy link
Member

@starbops starbops left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@TomTucka
Copy link

TomTucka commented Apr 5, 2024

@starbops Any idea when this could be merged?

@FrankYang0529 FrankYang0529 changed the title fix: add UntaggedNetwork and remove Custom fix: add UntaggedNetwork and remove NetworkTypeCustom Apr 9, 2024
Copy link
Member

@bk201 bk201 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bk201 bk201 merged commit 1506fdd into harvester:master Apr 9, 2024
1 check passed
@FrankYang0529 FrankYang0529 deleted the HARV-5226 branch April 9, 2024 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants