-
Notifications
You must be signed in to change notification settings - Fork 829
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
Terraform make targets: Switch from the plane structure to module terraform config #1375
Terraform make targets: Switch from the plane structure to module terraform config #1375
Conversation
Build Succeeded 👏 Build Id: 5200dc4f-676a-4528-b4b1-4e7562d8d0c9 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
60ec5ab
to
7939c4e
Compare
Fixed next error which was due to the wrong
|
Build Succeeded 👏 Build Id: 5a00f7a1-bc53-417a-844e-19eb2782fe2c The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
7939c4e
to
8cc0177
Compare
Build Failed 😱 Build Id: 69395431-12f7-4312-a039-e952672f91d2 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
8cc0177
to
e01c621
Compare
Build Succeeded 👏 Build Id: a6e8cff3-0838-4d0c-bccb-73d848cba06d The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Curious - why can't be re-apply the full terraform config each and every time? I.e. have a single If the cluster already exists, terraform is going to ignore it, and then the helm install will get updated as expected. If the cluster doesn't exist, it will create it anyway. We actually can accumulate several commands into one? WDYT? |
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
Build Succeeded 👏 Build Id: b2c6f68e-a8c8-4505-b4be-756948517f3c The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
c3d66f4
to
5661a24
Compare
5661a24
to
5ae7df1
Compare
Build Succeeded 👏 Build Id: daa1a447-329e-465e-af7d-05c1251fd0cd The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 96bb0eec-a568-4bbe-8060-7e40b88aa49a The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
5ae7df1
to
4ddb138
Compare
577084f
to
7684671
Compare
Resolved cluster name 🏷 |
Build Succeeded 👏 Build Id: 45d8b741-bd65-457d-93dd-ae44edb3b5a9 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Failed 😱 Build Id: e7187733-cb40-4c42-bec8-456d5343e04c To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
57a7f27
to
ac6e412
Compare
Build Succeeded 👏 Build Id: 8198f4c9-b4e6-41be-b9b4-d83da92349cd The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: 37ffb198-df76-4f19-b632-541002905c74 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
gcloud-terraform-install: | ||
ifndef GCP_PROJECT | ||
$(eval GCP_PROJECT=$(shell sh -c "gcloud config get-value project 2> /dev/null")) | ||
endif | ||
$(DOCKER_RUN) bash -c ' \ | ||
cd $(mount_path)/install/terraform && terraform apply -auto-approve -var agones_version="$(VERSION)" -var image_registry="$(REGISTRY)" \ | ||
cd $(mount_path)/build/terraform/gke && terraform apply -auto-approve -var agones_version="$(VERSION)" -var image_registry="$(REGISTRY)" \ |
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.
Should this target install the latest compiled version, like Make install ?
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.
Yes, it already is doing that. Latest compiled version got set using
terraform apply [...] -var agones_version="$(VERSION)"
this line
GCP_CLUSTER_NAME=test-cl2 make gcloud-terraform-install
docker run --rm -v /Users/alekser/go/src/github.com/agones/agones/build//.config/gcloud:/root/.config/gcloud -v ~/.kube/:/root/.kube -v ~/.helm:/root/.helm -v /Users/alekser/go/src
/github.com/agones/agones:/go/src/agones.dev/agones -v /Users/alekser/go/src/github.com/agones/agones/build//.gomod:/go/pkg/mod -v /Users/alekser/go/src/github.com/agones/agones/bu
ild//.gocache:/root/.cache/go-build -e "KUBECONFIG=/root/.kube/config" -e "GO111MODULE=on" -w /go/src/agones.dev/agones agones-build:8f473e7ff2 bash -c ' \
cd /go/src/agones.dev/agones/build/terraform/gke && terraform apply -auto-approve -var agones_version="1.5.0-39911ef" -var image_registry="gcr.io/agones-images" \
-var pull_policy=""Always"" \
-var always_pull_sidecar="true"
-var agones_version="1.5.0-39911ef"
which is current short hash of the latest commit.
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.
After that we are doing the same as in make install
agones/install/terraform/modules/helm/helm.tf
Lines 91 to 105 in 5413b28
# Use terraform of the latest >=0.12 version | |
values = [ | |
"${length(var.values_file) == 0 ? "" : file("${var.values_file}")}", | |
] | |
set { | |
name = "crds.CleanupOnDelete" | |
value = var.crd_cleanup | |
} | |
set { | |
name = local.tag_name | |
value = var.agones_version | |
} |
which is setting helm parameter "agones.image.tag" to be an up to date
$(VERSION)
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.
Tested this changes and the resulted controller pod uses correct image:
Controlled By: ReplicaSet/agones-controller-55d9bdd8f7 [50/706]
Containers:
agones-controller:
Container ID: docker://0a986a93103382805c304f837c68449719d47ac4390058effd1052e47719d466
Image: gcr.io/agones-images/agones-controller:1.5.0-39911ef
Registry could be changed by a REGISTRY
make variable.
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.
Awesome - thanks for the explanation!
Build Failed 😱 Build Id: 1a8f4bee-c268-47be-bf0e-9ef4928a2e71 To get permission to view the Cloud Build view, join the agones-discuss Google Group. |
For development installation using make target.
Refactor terraform.mk calls.
Remove Helm Release first and wait for 60 seconds so that forwarding rule and firewall rules would be cleaned up properly by the GCE controller
b8ce72b
to
f6212c2
Compare
Added
Will add Feature Gates variable into helm in the upcoming Pull Request. |
Now clusters created using gcloud and terraform would have different names.
f6212c2
to
51f0933
Compare
Build Succeeded 👏 Build Id: eefa6ead-cc8c-48b2-9b02-2df35f91dd36 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
Build Succeeded 👏 Build Id: a08f2bc5-dfc9-4c26-840d-6dddeccd4b55 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website (the last 30 builds are retained): To install this version:
|
gcloud-terraform-install: | ||
ifndef GCP_PROJECT | ||
$(eval GCP_PROJECT=$(shell sh -c "gcloud config get-value project 2> /dev/null")) | ||
endif | ||
$(DOCKER_RUN) bash -c ' \ | ||
cd $(mount_path)/install/terraform && terraform apply -auto-approve -var agones_version="$(VERSION)" -var image_registry="$(REGISTRY)" \ | ||
cd $(mount_path)/build/terraform/gke && terraform apply -auto-approve -var agones_version="$(VERSION)" -var image_registry="$(REGISTRY)" \ |
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.
Awesome - thanks for the explanation!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aLekSer, markmandel The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…raform config (googleforgames#1375) * Switch from the normal to module terraform config For development installation using make target. * Pass more variables to the module helm. Refactor terraform.mk calls. * Remove GKE cluster with a small delay after Helm Remove Helm Release first and wait for 60 seconds so that forwarding rule and firewall rules would be cleaned up properly by the GCE controller * Separate Terraform cluster by name - add variable Now clusters created using gcloud and terraform would have different names.
make targets for development
The main difference between
make gcloud-test-cluster
andmake gcloud-terraform-cluster
is that one use helm repo later one use local chart.There is an option to limit the number of resources created by using
terraform apply -target=resource
https://www.terraform.io/docs/commands/apply.html#target-resource
In the case when we want similar functionality to
make gcloud-test-cluster
andmake install
we should use-target=module.gke_cluster
and-target=module.helm_agones
Note that
values_file
andchart
variables are there to have oneHelm.tf
module performing two functions:For #657 and #1373