[GKE Hub]: Add Fleet Resource #8711
Conversation
|
Hello! I am a robot. It looks like you are a: @zli82016, 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. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 10 files changed, 1481 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
members = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
member = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_policy" "primary" {
policy_data = # value needed
}
|
Tests analyticsTotal tests: Action takenFound 16 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccBigtableAppProfile_bigtableAppProfileAnyclusterExample|TestAccBigtableAppProfile_bigtableAppProfileMulticlusterExample|TestAccBigtableAppProfile_bigtableAppProfileSingleclusterExample|TestAccDataplexDatascan_dataplexDatascanFullProfileExample|TestAccDataplexDatascan_dataplexDatascanBasicQualityExample|TestAccGKEHubFleetIamBindingGenerated|TestAccGKEHubFleet_gkehubFleetBasicExample_update|TestAccGKEHubFleet_gkehubFleetBasicExample|TestAccGKEHubFleetIamPolicyGenerated|TestAccGKEHubFleetIamMemberGenerated|TestAccGKEHubMembershipIamBindingGenerated|TestAccGKEHubMembership_gkehubMembershipIssuerExample|TestAccGKEHubMembershipIamMemberGenerated|TestAccGKEHubMembershipIamPolicyGenerated|TestAccGKEHubMembership_gkehubMembershipBasicExample|TestAccGKEHub2MembershipRBACRoleBinding_gkehubMembershipRbacRoleBindingBasicExample |
|
Rerun these tests in REPLAYING mode to catch issues
|
|
@zli82016 the fleet resource is a per-project singleton. Because of historical reasons, Creating a GKEHub membership will implicitly create the Fleet resource in the same project. Some of the tests that failed, failed because of an "existing fleet." Is there anyway to ensure the project is in a clean starting state before running these ones? |
zli82016
requested review from
a team and
NickElliot
and removed request for
zli82016 and
a team
|
I will be OOO office from tomorrow. Handoff the PR to @NickElliot . Thanks, Nick. |
There was a problem hiding this comment.
To make sure I'm understanding this correctly -- a fleet is not directly created by users, but done by default through enabling the membership? Can this default be deleted and manually created separately, or is it something that exists in perpetuity? It might need a pretty nonstandard test case and I'll verify with my team what would be the right way to handle this as a CI test depending on the specifics.
Also, the test failures in CI happened because your build failed rather than those singleton related issues:
google/services/gkehub/resource_gke_hub_fleet_test.go:10:2: no required module provides package github.com/hashicorp/terraform-provider-google-beta/google-beta/acctest
if you could go ahead and add that import 🙂
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 10 files changed, 1481 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
members = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
member = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_policy" "primary" {
policy_data = # value needed
}
|
Tests analyticsTotal tests: Action takenFound 13 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataprocJobIamPolicy|TestAccGKEHubFleetIamBindingGenerated|TestAccGKEHubFleetIamPolicyGenerated|TestAccGKEHubFleet_gkehubFleetBasicExample_update|TestAccGKEHubFleet_gkehubFleetBasicExample|TestAccGKEHubFleetIamMemberGenerated|TestAccGKEHubMembership_gkehubMembershipBasicExample|TestAccGKEHubMembershipIamBindingGenerated|TestAccGKEHubMembership_gkehubMembershipIssuerExample|TestAccGKEHubMembershipIamMemberGenerated|TestAccGKEHubMembershipIamPolicyGenerated|TestAccGKEHub2MembershipRBACRoleBinding_gkehubMembershipRbacRoleBindingBasicExample|TestAccHealthcareDatasetIamPolicy |
|
Rerun these tests in REPLAYING mode to catch issues
|
|
bumping in case you missed the question above* |
|
@NickElliot sorry for the delay. I was OOO the past week.
Not quite. For context, there are two resources here having the resource names paths:
A fleet can be managed individually with al CRUD APIs with the caveat that it cannot be deleted until all memberships in it have been deleted first. My concern is magic modules is using the same testing project for membership and fleet tests and they may collide. In fact I'm seeing some already. Additionally the conflicting resources are in different apis v1beta and v1beta1, so we have two products (gkehub and gkehub2) and handle that. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Breaking Change Detection FailedThe breaking change detector crashed during execution. This is usually due to the downstream provider(s) failing to compile. Please investigate or follow up with your reviewer. Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 10 files changed, 1481 insertions(+), 3 deletions(-)) |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 11 files changed, 1799 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 10 files changed, 1481 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
members = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
member = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_policy" "primary" {
policy_data = # value needed
}
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 10 files changed, 1481 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
members = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
member = # value needed
role = # value needed
}
Resource: resource "google_gke_hub_fleet_iam_policy" "primary" {
policy_data = # value needed
}
|
Tests analyticsTotal tests: Action takenFound 8 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataplexDatascan_dataplexDatascanBasicQualityExample|TestAccDataplexDatascan_dataplexDatascanFullProfileExample|TestAccGKEHub2Fleet_gkehubFleetBasicExample_update|TestAccGKEHub2Fleet_gkehubFleetBasicExample|TestAccGKEHub2FleetIamBindingGenerated|TestAccGKEHub2FleetIamPolicyGenerated|TestAccGKEHub2FleetIamMemberGenerated|TestAccHealthcareDatasetIamPolicy |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 9 files changed, 1472 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
member = # value needed
role = # value needed
}
|
Tests analyticsTotal tests: Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccGKEHub2FleetIamBindingGenerated|TestAccGKEHub2FleetIamPolicyGenerated|TestAccGKEHub2FleetIamMemberGenerated|TestAccDataSourceGoogleServiceAccountJwt |
|
|
|
@NickElliot that did the trick thanks! There is a panic happening in my IAM resource test though. I can't quite figure that out. Any ideas? |
There was a problem hiding this comment.
Sorry for the long delay on this review -- extremely busy around the 5.0.0 release
the problematic line is
m, err := tpgresource.GetImportIdQualifiers([]string{"projects/(?P<project>[^/]+)/locations/(?P<location>[^/]+)/fleets/default", "(?P<project>[^/]+)/(?P<location>[^/]+)", "(?P<location>[^/]+)"}, d, config, d.Get("id").(string))
in the GKEHub2FleetIamUpdaterProducer -- in otherwords d.Get("id") is returning a null value which is causing the panic error during the type conversion.
This led me to check if something was wrong with the IAM policies -- and saw that in the GKE Fleet documentation I dont see anything (i.e. GetIamPolicy or SetIamPolicy methods) there for Fleets. Does this resource actually need to be configured for IAM support?
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 9 files changed, 1442 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
member = # value needed
role = # value needed
}
|
Tests analyticsTotal tests: Action takenFound 4 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccGKEHub2FleetIamPolicyGenerated|TestAccGKEHub2FleetIamMemberGenerated|TestAccGKEHub2FleetIamBindingGenerated|TestAccDataSourceGoogleServiceAccountJwt |
|
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 9 files changed, 1441 insertions(+), 3 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet_iam_binding" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
}
Resource: resource "google_gke_hub_fleet_iam_member" "primary" {
condition {
description = # value needed
expression = # value needed
title = # value needed
}
member = # value needed
role = # value needed
}
|
Tests analyticsTotal tests: Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccGKEHub2FleetIamBindingGenerated|TestAccGKEHub2FleetIamMemberGenerated|TestAccGKEHub2FleetIamPolicyGenerated |
|
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 5 files changed, 808 insertions(+), 2 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet" "primary" {
display_name = # value needed
}
|
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 5 files changed, 808 insertions(+), 2 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: resource "google_gke_hub_fleet" "primary" {
display_name = # value needed
}
|
Tests analyticsTotal tests: Action takenFound 3 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataprocJobIamPolicy|TestAccDataSourceGoogleServiceAccountIdToken_impersonation|TestAccDataSourceGoogleServiceAccountJwt |
|
Rerun these tests in REPLAYING mode to catch issues
|
|
@NickElliot Ahh, yeah sorry about that! I didn't realize they weren't implemented. Everything looks good now! |
Adds a new GKEHub resource "Fleet." This is a singleton resource such that a user may have exactly one named "default" inside each project. i.e. the resource name
project/{project_id}/location/{global}/fleets/default.b/296461330
If this PR is for Terraform, I acknowledge that I have:
make testandmake lintin the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)