provider/kubernetes: Add support for resource_quota #13914
Conversation
There was a problem hiding this comment.
@radeksimko looking good - have a few questions inline but nothing alarming
| func resourceKubernetesResourceQuotaCreate(d *schema.ResourceData, meta interface{}) error { | ||
| conn := meta.(*kubernetes.Clientset) | ||
|
|
||
| metadata := expandMetadata(d.Get("metadata").([]interface{})) |
There was a problem hiding this comment.
Why does expandMetadata not return error status whereas expandResourceQuoteSpec does? Is there no change metadata could not return an error?
There was a problem hiding this comment.
That's merely because expandResourceQuoteSpec has to parse "resource list" and that may error out, expandMetadata doesn't have to parse anything like that.
https://github.com/hashicorp/terraform/pull/13914/files#diff-886d9a6a55a7569117aaf167990fed25R223
| log.Printf("[INFO] Submitted new resource quota: %#v", out) | ||
| d.SetId(buildId(out.ObjectMeta)) | ||
|
|
||
| err = resource.Retry(1*time.Minute, func() *resource.RetryError { |
There was a problem hiding this comment.
It is plenty of time based on my testing - it is usually ready within a few seconds.
| log.Printf("[INFO] Reading resource quota %s", name) | ||
| resQuota, err := conn.CoreV1().ResourceQuotas(namespace).Get(name) | ||
| if err != nil { | ||
| log.Printf("[DEBUG] Received error: %#v", err) |
There was a problem hiding this comment.
Can this be manually deleted? If so, can we catch it here?
There was a problem hiding this comment.
That's why there's resourceKubernetesResourceQuotaExists 😉
| } | ||
|
|
||
| func resourceKubernetesResourceQuotaUpdate(d *schema.ResourceData, meta interface{}) error { | ||
| conn := meta.(*kubernetes.Clientset) |
There was a problem hiding this comment.
Even if it's only spec that changes here, we are going to update the metadata - correct?
There was a problem hiding this comment.
ops := patchMetadata("metadata.0.", "/metadata/", d) should return empty slice of PatchOperations if it has no changes and if we have reached the Update() function it's clear that we have something to update - either spec or metadata, so it is there just to initialize the slice, really.
I could change it to something like
ops := make([]PatchOperations, 0, 0)
if d.HasChange("metadata") {
ops = append(ops, patchMetadata("metadata.0.", "/metadata/", d)
}if you think it's better?
|
Your answers sound good :) LGTM! |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
ghost
locked and limited conversation to collaborators
Test plan
Related upstream bug: kubernetes/kubernetes#44539