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

Add new resource netapp volumes quotaRules #12665

Merged
merged 13 commits into from
Feb 6, 2025

Conversation

Mehul3217
Copy link
Contributor

@Mehul3217 Mehul3217 commented Dec 28, 2024

Release Note Template for Downstream PRs (will be copied)

See Write release notes for guidance.

`google_netapp_volume_quota_rule`

@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 ( 7 files changed, 1091 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1091 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 96 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 128 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_netapp_volume_quota_rule (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_netapp_volume_quota_rule" "primary" {
  description = # value needed
  labels      = # value needed
  target      = # value needed
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 0
Passed tests: 0
Skipped tests: 0
Affected tests: 0

Click here to see the affected service packages
  • netapp
#### Non-exercised tests

🔴 Tests were added that are skipped in VCR:

  • TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample
  • TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample
    🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR.

View the build log

@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 ( 7 files changed, 1176 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1176 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 116 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 130 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_netapp_volume_quota_rule (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_netapp_volume_quota_rule" "primary" {
  description = # value needed
  labels      = # value needed
  target      = # value needed
}

1 similar comment
@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 ( 7 files changed, 1176 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1176 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 116 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 130 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_netapp_volume_quota_rule (1 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_netapp_volume_quota_rule" "primary" {
  description = # value needed
  labels      = # value needed
  target      = # value needed
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 22
Passed tests: 20
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages
  • netapp

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample [Error message] [Debug log]
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update [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

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 22
Passed tests: 20
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages
  • netapp

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample [Error message] [Debug log]
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update [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

@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 ( 7 files changed, 1176 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1176 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 116 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 130 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 22
Passed tests: 20
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages
  • netapp

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample [Error message] [Debug log]
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update [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

- 'DEFAULT_USER_QUOTA'
- 'DEFAULT_GROUP_QUOTA'
- name: 'diskLimitMib'
type: String
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be type "Integer" so TF checks if it is a valid number?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

name = "testvolumequotaRule%{random_suffix}"
description = "This is a updated description"
type = "DEFAULT_USER_QUOTA"
disk_limit_mib = 50
Copy link
Contributor

Choose a reason for hiding this comment

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

Change the actual value too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

protocols = ["NFSV3"]
}

resource "google_netapp_volume_quota_rule" "test_quotaRule" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add test for all 4 types of quotas?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@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 ( 7 files changed, 1262 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1262 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 116 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 130 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_netapp_volume_quota_rule (3 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_netapp_volume_quota_rule" "primary" {
  labels = # value needed
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 22
Passed tests: 20
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages
  • netapp

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🔴 Tests failed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample [Error message] [Debug log]
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update [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

@Mehul3217
Copy link
Contributor Author

/gcbrun

@Mehul3217 Mehul3217 marked this pull request as ready for review January 28, 2025 08:43
@github-actions github-actions bot requested a review from BBBmau January 28, 2025 08:44
Copy link

github-actions bot commented Jan 28, 2025

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

@SirGitsalot, 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.

Copy link
Contributor

@okrause okrause left a comment

Choose a reason for hiding this comment

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

Thank you! Looks pretty good. Suggested a few small changes. I learned that it pay to be explicit in descriptions.

name: 'VolumeQuotaRule'
api_resource_type_kind: QuotaRule
description: |
<TO BE FILLED>
Copy link
Contributor

Choose a reason for hiding this comment

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

QuotaRule specifies the maximum capacity a user or group can use within a volume. They can be used for creating default and individual quota rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

<TO BE FILLED>
references:
guides:
'Documentation': <TBD>
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

references:
guides:
'Documentation': <TBD>
api: <TBD>
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- name: 'target'
type: String
description: |
The quota rule applies to the specified user or group, identified by a Unix UID/GID, Windows SID, or null for default.
Copy link
Contributor

Choose a reason for hiding this comment

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

The quota rule applies to the specified user or group.
Valid targets for volumes with NFS protocol enabled:

  • UNIX UID for individual user quota
  • UNIX GID for individual group quota
    Valid targets for volumes with SMB protocol enabled:
  • Windows SID for individual user quota

Leave empty for default quotas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

- name: 'diskLimitMib'
type: Integer
description:
The maximum allowed disk space in MiB.
Copy link
Contributor

Choose a reason for hiding this comment

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

"disk space" is a somehow outdated term. I would prefer to use "capacity" instead. Unfortunately, the term disk space is encoded in the field parameter. Now the question is: use "disk space" in parameter and description, or change description to "capacity"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, changed to capacity

Copy link

@BBBmau This PR has been waiting for review for 3 weekdays. Please take a look! Use the label disable-review-reminders to disable these notifications.

@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 ( 7 files changed, 1274 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1274 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 116 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 130 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_netapp_volume_quota_rule (3 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_netapp_volume_quota_rule" "primary" {
  labels = # value needed
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 23
Passed tests: 21
Skipped tests: 0
Affected tests: 2

Click here to see the affected service packages
  • netapp

Action taken

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

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🔴 Tests failed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update [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

@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 ( 7 files changed, 1310 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1310 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 116 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 130 insertions(+))

Missing test report

Your PR includes resource fields which are not covered by any test.

Resource: google_netapp_volume_quota_rule (3 total tests)
Please add an acceptance test which includes these fields. The test should include the following:

resource "google_netapp_volume_quota_rule" "primary" {
  labels = # value needed
}

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 23
Passed tests: 22
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages
  • netapp

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
  • TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@Mehul3217
Copy link
Contributor Author

@BBBmau a gentle reminder to review this PR please

Copy link

github-actions bot commented Feb 4, 2025

@GoogleCloudPlatform/terraform-team @BBBmau This PR has been waiting for review for 1 week. Please take a look! Use the label disable-review-reminders to disable these notifications.

@melinath
Copy link
Member

melinath commented Feb 4, 2025

@modular-magician reassign-reviewer

@github-actions github-actions bot requested a review from SirGitsalot February 4, 2025 17:59
@melinath
Copy link
Member

melinath commented Feb 4, 2025

@SirGitsalot could you take a look at this today?

Copy link
Member

@SirGitsalot SirGitsalot left a comment

Choose a reason for hiding this comment

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

One small comment, otherwise LGTM

mmv1/products/netapp/VolumeQuotaRule.yaml Outdated Show resolved Hide resolved
@github-actions github-actions bot requested a review from SirGitsalot February 5, 2025 04:05
@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 ( 7 files changed, 1319 insertions(+), 2 deletions(-))
google-beta provider: Diff ( 7 files changed, 1319 insertions(+), 2 deletions(-))
terraform-google-conversion: Diff ( 1 file changed, 116 insertions(+))
Open in Cloud Shell: Diff ( 4 files changed, 130 insertions(+))

@modular-magician
Copy link
Collaborator

Tests analytics

Total tests: 23
Passed tests: 22
Skipped tests: 0
Affected tests: 1

Click here to see the affected service packages
  • netapp

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
  • TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update

Get to know how VCR tests work

@modular-magician
Copy link
Collaborator

🟢 Tests passed during RECORDING mode:
TestAccNetappVolumeQuotaRule_netappVolumeQuotaRuleBasicExample_update [Debug log]

🟢 No issues found for passed tests after REPLAYING rerun.


🟢 All tests passed!

View the build log or the debug log for each test

@melinath melinath removed request for okrause and BBBmau February 6, 2025 16:56
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.

5 participants