-
Notifications
You must be signed in to change notification settings - Fork 228
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 fleet_workspace parameter #702
Conversation
This closes rancher/rancher rancher#502 by adding a new parameter that allows setting of the FleetWorkspaceName value. Signed-off-by: Stephen Gran <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sgran, thanks for the PR. Please take a look to the inline comments.
Also, this PR is breaking tf provider acceptance tests, due to fleet_workspace
argument is just available at Rancher v2.5 or above. The acceptance tests are testing the Rancher upgrade from 2.3.x, to 2.4.x and to 2.5.x, so just the cluster definition for 2.5.x should contains fleet_workspace
argument. The upgrade tests are defined here https://github.com/rancher/terraform-provider-rancher2/blob/master/rancher2/0_provider_upgrade_test.go .
@@ -536,6 +536,7 @@ The following arguments are supported: | |||
* `annotations` - (Optional/Computed) Annotations for the Cluster (map) | |||
* `labels` - (Optional/Computed) Labels for the Cluster (map) | |||
* `windows_prefered_cluster` - (Optional) Windows preferred cluster. Default: `false` (bool) | |||
* `fleet_workspace` - (Optional/Computed) Fleet Workspace for the Cluster (string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This field is just available on rancher v2.5.x or above. It should be commented at docs.
Optional: true, | ||
Default: "fleet-default", | ||
Description: "Fleet Workspace for Cluster", | ||
ForceNew: false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default ForceNew
is false, not required.
"fleet_workspace": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
Default: "fleet-default", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Due to fleet_workspace
is a new argument on rancher2_cluster
, i think'd be more secure for tfp upgrade, set this argument as Computed: true
and removing default value.
err = d.Set("fleet_workspace", in.FleetWorkspaceName) | ||
if err != nil { | ||
return err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For setting string arguments, not required to check for error.
+ d.Set("fleet_workspace", in.FleetWorkspaceName)
- err = d.Set("fleet_workspace", in.FleetWorkspaceName)
- if err != nil {
- return err
- }
This PR has been superseded by #726 |
This closes rancher/terraform-provider-rancher2 #502 by adding a new parameter that allows
setting of the FleetWorkspaceName value.
Signed-off-by: Stephen Gran [email protected]