K8s #180
K8s #180
Conversation
|
CDS Report terraform-provider-ovh-check#81.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#81.0 ✔
|
There was a problem hiding this comment.
thanks for this PR, this is great work.
this is probably going to be merged in the next release. there are still some stuff to be done yet.
i'll make general comments here and more precise ones inline.
resource naming
resources/datasources should be named after their api path
so the nodepool is should be renamed:
ovh_cloud_project_kube_node_pool -> ovh_cloud_project_kube_nodepool
file naming
first global remark: some files should be renamed accordlingly to the resources/datasources they define.
eg: ovh/resource_ovh_cloud_kubernetes_cluster.go -> ovh/resource_ovh_cloud_project_kube.go
ovh/data_source_ovh_cloud_kubernetes_cluster.go
ovh/data_source_ovh_cloud_kubernetes_cluster_test.go
ovh/resource_ovh_cloud_kubernetes_cluster.go
ovh/resource_ovh_cloud_kubernetes_node_pool_test.go
ovh/resource_ovh_cloud_kubernetes_node_pool.go
ovh/resource_ovh_cloud_kubernetes_cluster_test.go
documentation
documentation files are missing. there should be a doc file for each datasource/resource file added
ovh/data_source_ovh_cloud_project_kube.go
ovh/resource_ovh_cloud_project_kube.go
ovh/resource_ovh_cloud_project_kube_node_pool.go
commits
could you squash your commits before we merge ?
|
CDS Report terraform-provider-ovh-check#85.0 ✔
|
|
CDS Report terraform-provider-ovh-check#86.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#86.0 ✔
|
|
CDS Report terraform-provider-ovh-check#92.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#92.0 ✔
|
|
CDS Report terraform-provider-ovh-check#93.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#93.0 ✔
|
|
CDS Report terraform-provider-ovh-check#123.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#123.0 ✘
|
|
CDS Report terraform-provider-ovh-check#124.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#124.0 ✔
|
|
CDS Report terraform-provider-ovh-check#126.0 ✘
|
|
CDS Report terraform-provider-ovh-testacc#126.0 ✘
|
|
CDS Report terraform-provider-ovh-check#126.1 ✘
|
|
CDS Report terraform-provider-ovh-testacc#126.1 ✘
|
|
CDS Report terraform-provider-ovh-check#131.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#131.0 ✔
|
ovh/helpers/helpers.go
Outdated
|
|
||
| // WaitAvailable wait for a ressource to become available in the API (aka non 404) | ||
| func WaitAvailable(client *ovh.Client, endpoint string) error { | ||
| return resource.Retry(30*time.Minute, func() *resource.RetryError { |
There was a problem hiding this comment.
i'm ok with adding this func in general helpers. but i think that 30min is a much too high timeout
if ever a resource should be created by an api and that it doesn"t converge faster that 1 minute. it's an issue with the endpoint. so i'd probably delegate the set of timeout value to the caller, and add a timeout arg.
what do you think?
| Type: schema.TypeString, | ||
| Computed: true, | ||
| }, | ||
| "kubeconfig": { |
There was a problem hiding this comment.
there's an issue here.
i don't think, the way readCloudProjectKube is implemented, that this attribute is ever set.
so either we remove it from the datasource.
Or, readCloudProjectKube has to be changed no ?
|
CDS Report terraform-provider-ovh-check#167.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#167.0 ✘
|
|
CDS Report terraform-provider-ovh-testacc#167.1 ✔
|
- data/ovh_cloud_project_kube - ovh_cloud_project_kube - ovh_cloud_project_kube_nodepool
|
CDS Report terraform-provider-ovh-check#169.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#169.0 ✘
|
|
CDS Report terraform-provider-ovh-testacc#169.1 ■ |
1 similar comment
|
CDS Report terraform-provider-ovh-testacc#169.1 ■ |
|
CDS Report terraform-provider-ovh-check#169.0 ✔
|
|
CDS Report terraform-provider-ovh-testacc#169.2 ✘
|
|
CDS Report terraform-provider-ovh-testacc#169.3 ✘
|
|
CDS Report terraform-provider-ovh-testacc#169.4 ✔
|
|
finally thanks to all of you @PhilippeVienne @d33d33 |
No description provided.