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

Adds Deployment Data Source #75

Merged
merged 6 commits into from
Nov 14, 2018

Conversation

itmecho
Copy link

@itmecho itmecho commented Oct 31, 2018

I had a need to load a deployment from kubernetes to use the currently running container image in my terraform resource. This PR adds the kubernetes_deployment data source.

I ran the acceptance test against minikube and have used the data source with my current kubernetes setup (running on AWS) and it seems to be working as intended.

This is my first time working in this repo so any tips/errors I made would be great to know!

@sl1pm4t
Copy link
Owner

sl1pm4t commented Nov 13, 2018

Hi @itmecho - thanks for the PR and sorry for the delay getting to it.

This looks good, but I have one suggetion that will make the datasource more easily maintainable going forward. We can leverage some code I added to the Google provider a while back, that can generate datasource schema definition from a resource:
See
https://github.com/terraform-providers/terraform-provider-google/blob/2.0.0/google/datasource_helpers.go

and this is an example datasource that uses the above:

https://github.com/terraform-providers/terraform-provider-google/blob/2.0.0/google/data_source_google_container_cluster.go

Are you OK to implement this?
Thanks

Copy link
Owner

@sl1pm4t sl1pm4t left a comment

Choose a reason for hiding this comment

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

Thanks!

@sl1pm4t sl1pm4t merged commit c47d4f1 into sl1pm4t:custom Nov 14, 2018
@itmecho
Copy link
Author

itmecho commented Nov 14, 2018

Awesome, Thanks!

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.

2 participants