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

Support IPV6_ONLY configurations for compute API #12283

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

karolgorc
Copy link
Contributor

@karolgorc karolgorc commented Nov 8, 2024

related to b/360733056

This provides support to set external and internal IPV6 addresses only for VM's and templates

I'm not sure if changing ipv6_access_config to Computed: true is a breaking change but it's needed because when providing external ipv6 subnetwork the field will be filled from API. CI tests should spot this if it is breaking but would love some feedback here

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

compute: `stack_type` can now be set to `IPV6_ONLY` on `google_compute_subnetwork`, `google_compute_instance`, `google_compute_instance_template` and `google_compute_region_instance_template`.

Copy link

github-actions bot commented Nov 8, 2024

Hello! I am a robot. Tests will require approval from a repository maintainer to run.

@slevenick, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look.

You can help make sure that review is quick by doing a self-review and by running impacted tests locally.

@modular-magician modular-magician added awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests service/compute-instances service/compute-vpc and removed awaiting-approval Pull requests that needs reviewer's approval to run presubmit tests labels Nov 8, 2024
@modular-magician
Copy link
Collaborator

Hi there, I'm the Modular magician. I've detected the following information about your changes:

Diff report

Your PR generated some diffs in downstreams - here they are.

google provider: Diff ( 8 files changed, 12 insertions(+), 9 deletions(-))
google-beta provider: Diff ( 8 files changed, 12 insertions(+), 9 deletions(-))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 1060
Passed tests: 986
Skipped tests: 73
Affected tests: 1

Click here to see the affected service packages
  • compute

Action taken

Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
  • TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Get to know how VCR tests work

@@ -541,7 +541,8 @@ func ResourceComputeInstance() *schema.Resource {

"ipv6_access_config": {
Type: schema.TypeList,
Optional: true,
Optional: true,
Computed: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we adding Computed on this field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As stated in PR's description, if the subnetwork given to the network_interface is a subnetwork of external IPv6 addresses ipv6_access_config will get filled from the API without user's interaction

resource "google_compute_subnetwork" "test" {
  name = "testing"
  network = google_compute_network.test.self_link
  stack_type = "IPV6_ONLY"
  ipv6_access_type = "EXTERNAL"
}

resource "google_compute_instance" "test" {
  name = "test"
  machine_type = "n2-standard-2"

  boot_disk {
    initialize_params {
      image = "debian-cloud/debian-11"
    }
  }

  network_interface {
    subnetwork = google_compute_subnetwork.test.self_link
    stack_type = "IPV6_ONLY"
  }
}

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy [Error message] [Debug log]

🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR.

View the build log or the debug log for each test

@karolgorc
Copy link
Contributor Author

What does this fail on?

TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Can't test this locally

Error: Error creating RegionSecurityPolicy: googleapi: Error 400: Invalid value for field 'resource.type': 'CLOUD_ARMOR_NETWORK'. Network Security Policies are not supported as part of the current Cloud Armor service tier., invalid

@slevenick
Copy link
Contributor

slevenick commented Nov 12, 2024

What does this fail on?

TestAccComputeInstanceNetworkIntefaceWithSecurityPolicy

Can't test this locally

Error: Error creating RegionSecurityPolicy: googleapi: Error 400: Invalid value for field 'resource.type': 'CLOUD_ARMOR_NETWORK'. Network Security Policies are not supported as part of the current Cloud Armor service tier., invalid

resource_compute_instance_test.go:3893: Step 5/5, expected an error with pattern, no match on: Error running apply: exit status 1

    Error: Error deleting old access_config: googleapi: Error 400: Invalid value for field 'accessConfig': 'external-nat'. Cannot delete an access config with a security policy set. Please remove the security policy first., invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants