Make it possible to remove backend from director #547
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Integralist
approved these changes
Feb 21, 2022
| // and in the next Terraform run the first director is updated while | ||
| // the second director is unchanged and a third director is added. | ||
| // In the final test, the first director is removed while the second | ||
| // director is unchanged and one backend for the third director is removed. | ||
| func TestAccFastlyServiceVCL_directors_basic(t *testing.T) { |
There was a problem hiding this comment.
❤️
I always appreciate when people take the time to provide a 'summary' of a test.
| @@ -301,3 +318,31 @@ func flattenDirectors(directorList []*gofastly.Director, directorBackendList []* | |||
| } | |||
| return dl | |||
| } | |||
|
|
|||
| func getDirectorBackendChange(d *schema.ResourceData, resource map[string]interface{}) (odb *schema.Set, ndb *schema.Set) { | |||
There was a problem hiding this comment.
Just to double check: are we not able to reuse
terraform-provider-fastly/fastly/diff.go
Line 52 in 256c95d
There was a problem hiding this comment.
The Diff method cannot replace getDirectorBackendChange(). To my understanding, Diff is handling a different issue:
- The Diff method takes two Set objects and converts them into a DiffResult object containing the Added, Modified, Deleted, and Unmodified lists.
- getDirectorBackendChange() gets Director Set objects from
*schema.ResourceData, iterate over them to find the changed Director, then extract Director Backend Set objects from the Director Set and return them. (Basically, it returns child Set objects extracted from parent Set objects)
| @@ -301,3 +318,31 @@ func flattenDirectors(directorList []*gofastly.Director, directorBackendList []* | |||
| } | |||
| return dl | |||
| } | |||
|
|
|||
| func getDirectorBackendChange(d *schema.ResourceData, resource map[string]interface{}) (odb *schema.Set, ndb *schema.Set) { | |||
| name := resource["name"] | |||
There was a problem hiding this comment.
I'd prefer to keep the variable initialisation near to where it is used (see the following suggested change).
Suggested change
| name := resource["name"] |
| return new(schema.Set) | ||
| } | ||
|
|
||
| odb = get(name.(string), od.(*schema.Set)) |
There was a problem hiding this comment.
Suggested change
| odb = get(name.(string), od.(*schema.Set)) | |
| name := resource["name"] | |
| odb = get(name.(string), od.(*schema.Set)) |
|
Also @hrmsk66 can you run |
Integralist
approved these changes
Feb 21, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #545
Fixes #546
Make it possible to remove backends from a director.
This is a proposed fix for #545
If the backend in a director block changes, get the changes from
*schema.ResourceDataand compute backends to be removed/added using Set'sDifferencemethod.Removing a director backend can result in a 404 error if the backend is removed from the service. I'm just ignoring the error in this fix since they don't mean anything useful to users.
I've also fixed an unrelated issue (#546) with the comment field in the director block which was causing the acceptance tests to fail.