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.

Sign up for GitHub

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

Jump to bottom

Wait_till option : normal #4139

Merged
merged 15 commits into from
Nov 9, 2022
Merged

Wait_till option : normal #4139

merged 15 commits into from
Nov 9, 2022

Conversation

attilatabori
Copy link
Contributor

@attilatabori attilatabori commented Nov 4, 2022 •
edited
Loading

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

tabori@Attilas-MacBook-Pro terraform-provider-ibm % make testacc TEST=./ibm/service/kubernetes TESTARGS='-run=TestAccIBMContainerVpcClusterEnvvar'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./ibm/service/kubernetes -v -run=TestAccIBMContainerVpcClusterEnvvar -timeout 700m
[…]
--- PASS: TestAccIBMContainerVpcClusterEnvvar (1560.43s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes      1561.907s

data source with classic cluster:
tabori@Attilas-MacBook-Pro terraform-provider-ibm % make testacc TEST=./ibm/service/kubernetes TESTARGS='-run=TestAccIBMContainer_ClusterConfigDataSourceBasic'   
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./ibm/service/kubernetes -v -run=TestAccIBMContainer_ClusterConfigDataSourceBasic -timeout 700m
[…]
--- PASS: TestAccIBMContainer_ClusterConfigDataSourceBasic (1052.15s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes      1053.532s


vpc data source:

tabori@Attilas-MacBook-Pro terraform-provider-ibm % make testacc TEST=./ibm/service/kubernetes TESTARGS='-run=TestAccIBMContainer_ClusterConfigDataSourceVpcBasic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./ibm/service/kubernetes -v -run=TestAccIBMContainer_ClusterConfigDataSourceVpcBasic -timeout 700m
[…]
2022-11-08T10:56:30.313+0100 [DEBUG] sdk.helper_resource: Finished TestCase: test_name=TestAccIBMContainer_ClusterConfigDataSourceVpcBasic
--- PASS: TestAccIBMContainer_ClusterConfigDataSourceVpcBasic (1334.31s)
PASS
ok      github.com/IBM-Cloud/terraform-provider-ibm/ibm/service/kubernetes      1335.692s

...

ocofaigh
ocofaigh reviewed Nov 4, 2022
View reviewed changes
Copy link
Contributor

@ocofaigh ocofaigh left a comment

Choose a reason for hiding this comment

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

What does "normal" mean? And I guess docs need to be updated for this change too

@z0za
Copy link
Contributor

z0za commented Nov 4, 2022

What does "normal" mean? And I guess docs need to be updated for this change too

"All worker nodes in a cluster are up and running. You can access the cluster and deploy apps to the cluster. This state is considered healthy and does not require an action from you."

https://cloud.ibm.com/docs/containers?topic=containers-cluster-states-reference#cluster-state-normal

yes, this needs to be added to the tf docs too.

@ocofaigh
Copy link
Contributor

ocofaigh commented Nov 4, 2022

So what is going to be the difference between "Normal" and "IngressReady"?
According to the docs:

IngressReady: Terraform marks the creation of your cluster complete when the cluster master and all worker nodes are in a ready state, and the Ingress subdomain is fully set up.

Is it simply that the "Normal" will not wait until the Ingress subdomain is fully set up?

@z0za
Copy link
Contributor

z0za commented Nov 4, 2022

yes, that is my understanding

@attilatabori attilatabori changed the title Cluster deployed Wait_till option : normal Nov 7, 2022
ocofaigh
ocofaigh reviewed Nov 7, 2022
View reviewed changes
Copy link
Contributor

@ocofaigh ocofaigh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

We should probably be consistent with naming here. The current values all use CamelCase (e.g. IngressReady) and looks like you are introducing a lower case value "normal". Should it be uppercase "Normal" ?

@attilatabori
Copy link
Contributor Author

attilatabori commented Nov 8, 2022 •
edited
Loading

We should probably be consistent with naming here. The current values all use CamelCase (e.g. IngressReady) and looks like you are introducing a lower case value "normal". Should it be uppercase "Normal" ?

it is seen in the test, but the value were already existing for cluster, it is converted to lowercase too. in the docs it is written in as Normal, and it is okay to use it this way.

z0za
z0za reviewed Nov 8, 2022
View reviewed changes
ibm/service/kubernetes/resource_ibm_container_cluster.go Outdated Show resolved Hide resolved
ibm/service/kubernetes/resource_ibm_container_cluster.go Outdated Show resolved Hide resolved
ibm/service/kubernetes/resource_ibm_container_vpc_cluster.go Outdated Show resolved Hide resolved
ibm/service/kubernetes/resource_ibm_container_vpc_cluster.go Outdated Show resolved Hide resolved
ibm/service/kubernetes/resource_ibm_container_vpc_cluster.go Outdated Show resolved Hide resolved
ibm/service/kubernetes/resource_ibm_container_vpc_cluster_test.go Outdated Show resolved Hide resolved
ibm/service/kubernetes/resource_ibm_container_vpc_cluster.go Outdated Show resolved Hide resolved
website/docs/r/container_vpc_cluster.html.markdown Outdated Show resolved Hide resolved
@hkantare hkantare merged commit 3150e2c into IBM-Cloud:master Nov 9, 2022
ocofaigh
ocofaigh reviewed Nov 9, 2022
View reviewed changes
website/docs/r/container_cluster.html.markdown
@@ -241,7 +241,7 @@ Review the argument references that you can specify for your resource.
- `version` - (Optional, String) The Kubernetes version that you want to update your worker nodes to.
- `worker_num`- (Optional, Integer) The number of worker nodes in your cluster. This attribute creates a worker node that is not associated with a worker pool. **Note**: Conflicts with `workers`.
- `wait_for_worker_update` - (Optional, Bool) Set to **true** to wait and update the Kubernetes version of worker nodes. **NOTE** Setting wait_for_worker_update to **false** is not recommended. Setting **false** results in upgrading all the worker nodes in the cluster at the same time causing the cluster downtime.
- `wait_till` - (Optional, String) The cluster creation happens in multi-stages. To avoid the longer wait times for resource execution.This argument in the resource will wait for the specified stage and complete the execution. The default stage value is `IngressReady`. The supported stages are `MasterNodeReady` Resource waits till the master node is ready. `OneWorkerNodeReady` Resource waits till one worker node is in to ready state. `IngressReady` Resource waits till the ingress-host and ingress-secret are available.
- `wait_till` - (Optional, String) The cluster creation happens in multi-stages. To avoid the longer wait times for resource execution.This argument in the resource will wait for the specified stage and complete the execution. The default stage value is `IngressReady`. The supported stages are `MasterNodeReady` Resource waits till the master node is ready. `OneWorkerNodeReady` Resource waits till one worker node is in to ready state. `Normal` Terraform marks the creation of your cluster complete when the cluster is in a [Normal](https://cloud.ibm.com/docs/containers?topic=containers-cluster-states-reference#cluster-state-normal) state. If you plan to do reading on the cluster from a datasource, use `Normal`. `IngressReady` Resource waits till the ingress-host and ingress-secret are available.
Copy link
Contributor

Choose a reason for hiding this comment

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

missing a space -> "execution.This"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@z0za z0za z0za left review comments

@ocofaigh ocofaigh ocofaigh left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@attilatabori @z0za @ocofaigh @hkantare