Added variable `skip_provisioners` to skip 'local-exec' #278

paulpalamarchuk · 2019-10-08T17:19:43Z

Fix Allow local-exec to be skipped #258
Add test simple_regional_skip_local_exec
Remove old upgrading guide from README's

kopachevsky · 2019-10-09T08:19:49Z

README.md

+## Upgrade to v3.0.0
+
+v3.0.0 is a breaking release. Refer to the
+[Upgrading to v3.0 guide][upgrading-to-v3.0] for details.
+
+## Upgrade to v2.0.0
+
+v2.0.0 is a breaking release. Refer to the
+[Upgrading to v2.0 guide][upgrading-to-v2.0] for details.
+
+## Upgrade to v1.0.0
+
+Version 1.0.0 of this module introduces a breaking change: adding the `disable-legacy-endpoints` metadata field to all node pools. This metadata is required by GKE and [determines whether the `/0.1/` and `/v1beta1/` paths are available in the nodes' metadata server](https://cloud.google.com/kubernetes-engine/docs/how-to/protecting-cluster-metadata#disable-legacy-apis). If your applications do not require access to the node's metadata server, you can leave the default value of `true` provided by the module. If your applications require access to the metadata server, be sure to read the linked documentation to see if you need to set the value for this field to `false` to allow your applications access to the above metadata server paths.
+
+In either case, upgrading to module version `v1.0.0` will trigger a recreation of all node pools in the cluster.
+


why it's showing up as new change?

Thank you for notice.
Removed old upgrading guides all README's. In addition to commit

kopachevsky · 2019-10-09T08:23:17Z

autogen/dns.tf

@@ -20,7 +20,7 @@
  Delete default kube-dns configmap
 *****************************************/
 resource "null_resource" "delete_default_kube_dns_configmap" {
-  count = local.custom_kube_dns_config || local.upstream_nameservers_config ? 1 : 0
+  count = (local.custom_kube_dns_config || local.upstream_nameservers_config) || var.skip_provisioners ? 1 : 0


need to add explanation of this in skip_provisioners var description

Updated description of the variable.

This logic is wrong.

If you skip provisioners, no local-execs should ever be initialized.

morgante · 2019-10-16T15:55:06Z

.kitchen.yml

@@ -131,3 +131,10 @@ suites:
      systems:
        - name: workload_metadata_config
          backend: local
+  - name: "simple_regional_skip_local_exec"


This doesn't need to be a separate suite, just add it to one of the existing ones.

Moved to simple_regional

morgante · 2019-10-16T15:55:30Z

autogen/README.md

@@ -122,22 +122,6 @@ Then perform the following commands on the root folder:
 - `terraform apply` to apply the infrastructure build
 - `terraform destroy` to destroy the built infrastructure

-## Upgrade to v3.0.0


Why did you remove this? Please rebase.

morgante · 2019-10-16T15:56:53Z

autogen/cluster.tf

@@ -352,6 +352,7 @@ resource "google_container_node_pool" "pools" {
 }

 resource "null_resource" "wait_for_cluster" {
+  count = var.skip_provisioners ? 1 : 0


If you skip provisioners, shouldn't this be count of 0? Please pay attention to quality of work and correct logic.

morgante · 2019-10-16T15:57:20Z

autogen/dns.tf

@@ -20,7 +20,7 @@
  Delete default kube-dns configmap
 *****************************************/
 resource "null_resource" "delete_default_kube_dns_configmap" {
-  count = local.custom_kube_dns_config || local.upstream_nameservers_config ? 1 : 0
+  count = (local.custom_kube_dns_config || local.upstream_nameservers_config) || var.skip_provisioners ? 1 : 0


This logic is wrong.

If you skip provisioners, no local-execs should ever be initialized.

kopachevsky · 2019-10-18T08:20:29Z

autogen/README.md

@@ -201,6 +201,7 @@ In either case, upgrading to module version `v1.0.0` will trigger a recreation o
 | resource\_usage\_export\_dataset\_id | The dataset id for which network egress metering for this cluster will be enabled. If enabled, a daemonset will be created in the cluster to meter network egress traffic. | string | `""` | no |
 | sandbox\_enabled | (Beta) Enable GKE Sandbox (Do not forget to set `image_type` = `COS_CONTAINERD` and `node_version` = `1.12.7-gke.17` or later to use it). | bool | `"false"` | no |
 | service\_account | The service account to run nodes as if not overridden in `node_pools`. The create_service_account variable default value (true) will cause a cluster-specific service account to be created. | string | `""` | no |
+| skip\_provisioners | Flag to skip local-exec provisioners. Does not affect if `stub_domains` or `upstream_nameservers` variable set. | bool | `"false"` | no |


change this one

* Fix terraform-google-modules#258 * Added test `simple_regional_skip_local_exec` * Remove old upgrading guide from README's

…al-exec

morgante · 2019-10-22T20:29:31Z

autogen/variables.tf

@@ -311,6 +311,11 @@ variable "cluster_resource_labels" {
  default     = {}
 }

+variable "skip_provisioners" {
+  type        = bool
+  description = "Flag to skip all local-exec provisioners. It breaks down `stub_domains` and `upstream_nameservers` variables functionality."


Suggested change

description = "Flag to skip all local-exec provisioners. It breaks down `stub_domains` and `upstream_nameservers` variables functionality."

description = "Flag to skip all local-exec provisioners. It breaks `stub_domains` and `upstream_nameservers` variables functionality."

morgante · 2019-10-24T18:54:12Z

scripts/wait-for-cluster.sh

@@ -15,10 +15,6 @@

 set -e

-if [ -n "${GOOGLE_APPLICATION_CREDENTIALS}" ]; then


Why are you removing this?

…al-exec

paulpalamarchuk force-pushed the add_skip_provisioners_variable_to_skip_local-exec branch from 4de5836 to 90f64f4 Compare October 8, 2019 17:36

paulpalamarchuk requested a review from kopachevsky October 8, 2019 17:57

paulpalamarchuk force-pushed the add_skip_provisioners_variable_to_skip_local-exec branch from 90f64f4 to 8263a68 Compare October 9, 2019 07:39

kopachevsky suggested changes Oct 9, 2019

View reviewed changes

paulpalamarchuk force-pushed the add_skip_provisioners_variable_to_skip_local-exec branch 3 times, most recently from 0e0defd to 8727e84 Compare October 9, 2019 12:21

paulpalamarchuk requested a review from kopachevsky October 9, 2019 12:21

kopachevsky previously approved these changes Oct 9, 2019

View reviewed changes

paulpalamarchuk marked this pull request as ready for review October 9, 2019 15:22

paulpalamarchuk requested review from aaron-lane, adrienthebo, Jberlinsky and morgante as code owners October 9, 2019 15:22

aaron-lane added the enhancement New feature or request label Oct 9, 2019

aaron-lane assigned paulpalamarchuk Oct 11, 2019

morgante suggested changes Oct 16, 2019

View reviewed changes

paulpalamarchuk dismissed kopachevsky’s stale review via 5388ada October 17, 2019 13:13

paulpalamarchuk force-pushed the add_skip_provisioners_variable_to_skip_local-exec branch 4 times, most recently from dc2448d to c13e003 Compare October 17, 2019 14:05

paulpalamarchuk requested a review from kopachevsky October 17, 2019 16:56

kopachevsky previously approved these changes Oct 18, 2019

View reviewed changes

paulpalamarchuk dismissed kopachevsky’s stale review via ce97874 October 18, 2019 09:41

paulpalamarchuk force-pushed the add_skip_provisioners_variable_to_skip_local-exec branch 3 times, most recently from 0dca148 to 198b191 Compare October 18, 2019 10:26

Added variable skip_provisioners to skip 'local-exec'

9c66273

* Fix terraform-google-modules#258 * Added test `simple_regional_skip_local_exec` * Remove old upgrading guide from README's

paulpalamarchuk force-pushed the add_skip_provisioners_variable_to_skip_local-exec branch from 198b191 to 9c66273 Compare October 18, 2019 10:44

Merge branch 'master' into add_skip_provisioners_variable_to_skip_loc…

55ce9e3

…al-exec

paulpalamarchuk requested review from kopachevsky and morgante October 18, 2019 10:51

aaron-lane self-assigned this Oct 19, 2019

Merge branch 'master' into add_skip_provisioners_variable_to_skip_loc…

eb09369

…al-exec

morgante reviewed Oct 22, 2019

View reviewed changes

Fix logic of skip_provisioners behavior

53ec7a9

paulpalamarchuk force-pushed the add_skip_provisioners_variable_to_skip_local-exec branch from da4b297 to 53ec7a9 Compare October 23, 2019 07:55

morgante mentioned this pull request Oct 23, 2019

Running on Terraform Cloud Fails due to gcloud/jq dependency #293

Closed

morgante assigned morgante and unassigned aaron-lane Oct 24, 2019

morgante suggested changes Oct 24, 2019

View reviewed changes

Merge branch 'master' into add_skip_provisioners_variable_to_skip_loc…

cb959e3

…al-exec

morgante approved these changes Oct 24, 2019

View reviewed changes

aaron-lane merged commit ec96266 into terraform-google-modules:master Oct 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added variable `skip_provisioners` to skip 'local-exec' #278

Added variable `skip_provisioners` to skip 'local-exec' #278

paulpalamarchuk commented Oct 8, 2019 •

edited

Loading

kopachevsky Oct 9, 2019

paulpalamarchuk Oct 9, 2019 •

edited

Loading

kopachevsky Oct 9, 2019

paulpalamarchuk Oct 9, 2019

morgante Oct 16, 2019

paulpalamarchuk Oct 24, 2019

morgante Oct 16, 2019

paulpalamarchuk Oct 24, 2019

morgante Oct 16, 2019

paulpalamarchuk Oct 24, 2019

morgante Oct 16, 2019

paulpalamarchuk Oct 24, 2019

morgante Oct 16, 2019

kopachevsky Oct 18, 2019

morgante Oct 22, 2019

paulpalamarchuk Oct 24, 2019

morgante Oct 24, 2019

	description = "Flag to skip all local-exec provisioners. It breaks down `stub_domains` and `upstream_nameservers` variables functionality."
	description = "Flag to skip all local-exec provisioners. It breaks `stub_domains` and `upstream_nameservers` variables functionality."

		@@ -15,10 +15,6 @@

		set -e

		if [ -n "${GOOGLE_APPLICATION_CREDENTIALS}" ]; then

Added variable skip_provisioners to skip 'local-exec' #278

Added variable skip_provisioners to skip 'local-exec' #278

Conversation

paulpalamarchuk commented Oct 8, 2019 • edited Loading

Choose a reason for hiding this comment

paulpalamarchuk Oct 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Added variable `skip_provisioners` to skip 'local-exec' #278

Added variable `skip_provisioners` to skip 'local-exec' #278

paulpalamarchuk commented Oct 8, 2019 •

edited

Loading

paulpalamarchuk Oct 9, 2019 •

edited

Loading