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 field for PersistentVolumeClaim resource #2644

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JaylonmcShan03
Copy link
Contributor

@JaylonmcShan03 JaylonmcShan03 commented Dec 9, 2024

Description

Addresses #2608

This pull request introduces the data_source field for the PersistentVolumeClaim resource.

Changes

  1. Added data_source field to the PersistentVolumeClaim schema.
  2. Implemented validation in the Create function to ensure the specified data_source PVC exists.
  3. Included the new field in flatten and expand method.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Output from acceptance testing:

2024-12-09T10:14:13.480-0600 [DEBUG] sdk.helper_resource: Called TestCase CheckDestroy: test_step_number=2 test_terraform_path=/opt/homebrew/bin/terraform test_name=TestAccKubernetesPersistentVolumeClaimV1_dataSource test_working_directory=/var/folders/bp/l16ph9cj7958ml9t254_zcv00000gn/T/plugintest3860028569
2024-12-09T10:14:13.482-0600 [DEBUG] sdk.helper_resource: Finished TestCase: test_name=TestAccKubernetesPersistentVolumeClaimV1_dataSource
--- PASS: TestAccKubernetesPersistentVolumeClaimV1_dataSource (3.21s)
PASS
ok      github.com/hashicorp/terraform-provider-kubernetes/kubernetes   3.824s
jaylon.mcshan@jaylon terraform-provider-kubernetes % 


2024-12-09T10:15:00.499-0600 [DEBUG] sdk.helper_resource: Started sdkv2 provider instance server: tf_provider_addr=registry.terraform.io/hashicorp/kubernetes test_name=TestAccKubernetesPersistentVolumeClaimV1_invalidDataSource test_terraform_path=/opt/homebrew/bin/terraform test_working_directory=/var/folders/bp/l16ph9cj7958ml9t254_zcv00000gn/T/plugintest1113338307 test_step_number=1
2024-12-09T10:15:00.558-0600 [DEBUG] sdk.helper_resource: Stopping providers: test_working_directory=/var/folders/bp/l16ph9cj7958ml9t254_zcv00000gn/T/plugintest1113338307 test_step_number=1 test_name=TestAccKubernetesPersistentVolumeClaimV1_invalidDataSource test_terraform_path=/opt/homebrew/bin/terraform
2024-12-09T10:15:00.560-0600 [DEBUG] sdk.helper_resource: Finished TestCase: test_name=TestAccKubernetesPersistentVolumeClaimV1_invalidDataSource
--- PASS: TestAccKubernetesPersistentVolumeClaimV1_invalidDataSource (0.51s)
PASS
ok      github.com/hashicorp/terraform-provider-kubernetes/kubernetes   1.248s
jaylon.mcshan@jaylon terraform-provider-kubernetes %
$ make testacc TESTARGS='-run=TestAccXXX'

...

Release Note

Release note for CHANGELOG:

...

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

@github-actions github-actions bot added the size/L label Dec 9, 2024
@JaylonmcShan03 JaylonmcShan03 marked this pull request as ready for review January 6, 2025 16:32
@JaylonmcShan03 JaylonmcShan03 requested a review from a team as a code owner January 6, 2025 16:32
Copy link
Contributor

@BBBmau BBBmau left a comment

Choose a reason for hiding this comment

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

although tests are passing it seems like we're missing two fields that are mentioned in the docs kind and apiGroup

It looks like the PR is assuming that both kind and apiGroup are setup in a way that sets up the dataSource as PVC but from the docs users have the option to input their own values (though this is alpha something to note)

I'm thinking we should stick more with how the api and manifest is formatted meaning instead of

    data_source {
      persistent_volume_claim {
        claim_name = "example-source-pvc"
      }
    }

we should do

    data_source {
        name = "example-source-pvc"
    }

leaving the other fields as defaults but not part of the schema. once they are moved out of alpha we can add them in. This method would prevent any extra worked needed later on while also preventing some unexpected changes for users that would use this.

Hope this makes sense, let me know your thoughts!

@github-actions github-actions bot added size/XL and removed size/L labels Jan 9, 2025
@JaylonmcShan03 JaylonmcShan03 force-pushed the add-data-source-to-pvc branch from 1d6b7ac to 9f7577f Compare January 9, 2025 02:44
@github-actions github-actions bot added size/L and removed size/XL labels Jan 9, 2025
@evilhamsterman
Copy link

@BBBmau The kind and apiGroup field is actually very important in the stable API. It is possible to specify the dataSource as either a VolumeSnapshot or a PVC. You need the kind field to differentiate in that case you have both with the same name.

https://kubernetes.io/docs/concepts/storage/persistent-volumes/#create-persistent-volume-claim-from-volume-snapshot
https://kubernetes.io/docs/concepts/storage/persistent-volumes/#create-persistent-volume-claim-from-an-existing-pvc

@BBBmau
Copy link
Contributor

BBBmau commented Jan 9, 2025

@BBBmau The kind and apiGroup field is actually very important in the stable API. It is possible to specify the dataSource as either a VolumeSnapshot or a PVC. You need the kind field to differentiate in that case you have both with the same name.

kubernetes.io/docs/concepts/storage/persistent-volumes#create-persistent-volume-claim-from-volume-snapshot kubernetes.io/docs/concepts/storage/persistent-volumes#create-persistent-volume-claim-from-an-existing-pvc

This makes sense, not sure how I glanced over that. Thanks for bringing it up.

cc: @JaylonmcShan03

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

Successfully merging this pull request may close these issues.

3 participants