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.

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

Issue 501: Expose node pool "shielded_instance_config" #506

Merged
merged 4 commits into from
May 5, 2020
Merged

Issue 501: Expose node pool "shielded_instance_config" #506

merged 4 commits into from
May 5, 2020

Conversation

c0ffeec0der
Copy link
Contributor

Expose node pool "shielded_instance_config" for module:

  • beta-private-cluster
  • beta-private-cluster-update-variant
  • beta-public-cluster
  • private-cluster
  • private-cluster-update-variant

Tested for beta-private-cluster-update-variant
test-result

@c0ffeec0der c0ffeec0der requested review from bharathkkb, Jberlinsky and a team as code owners May 3, 2020 11:42
Copy link
Member

@bharathkkb bharathkkb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
The changes need to made in autogen/main/cluster.tf.tmpl which is then propagated to all the other modules by running make build. More details here.

@c0ffeec0der
Copy link
Contributor Author

You're welcome

I have updated the autogen/main/cluster.tf.tmpl

However, running make build run to error:

docker run --rm -it
-v "C:/main/repos/terraform-google-kubernetes-engine":/workspace
gcr.io/cloud-foundation-cicd/cft/developer-tools:0
/bin/bash -c 'source /usr/local/bin/task_helper_functions.sh && generate_modules'
/workspace/test/task_helper_functions.sh: line 2: $'\r': command not found
/workspace/test/task_helper_functions.sh: line 16: $'\r': command not found
/workspace/test/task_helper_functions.sh: line 18: syntax error near unexpected token $'{\r'' 'workspace/test/task_helper_functions.sh: line 18: function download_acm() {

I tried executing:

  1. docker run --rm -it
    -v "C:/main/repos/terraform-google-kubernetes-engine":/workspace
    gcr.io/cloud-foundation-cicd/cft/developer-tools:0
    /bin/bash
  2. gsutil cp gs://config-management-release/released/latest/config-management-operator.yaml /workspace/acm.yaml

results in another error:

401 Anonymous caller does not have storage.objects.get access to config-management-release/released/latest/config-management-operator.yaml

Any advice on this?

@bharathkkb
Copy link
Member

bharathkkb commented May 4, 2020

@c0ffeec0der that seems odd, I was able to run it fine from your branch.
I think it has something to with newlines (\n vs \r\n) in Windows (which is what I am assuming you are using)? Could you try running it from a linux box or the cloud shell?
The script it references is here: https://github.com/GoogleCloudPlatform/cloud-foundation-toolkit/blob/master/infra/build/developer-tools/build/scripts/task_helper_functions.sh

@c0ffeec0der
Copy link
Contributor Author

Hey, you're right!

Previously i thought to solve it by executing inside the container and came that 401 error. But then, after reading your review, maybe it was just the line /r code. So I browsed around and found this thread:

https://stackoverflow.com/questions/11616835/r-command-not-found-bashrc-bash-profile

dos2unix command solved the issue and the tool worked!

I commited all the changes except for file task_helper_functions.sh, because it was modified by dos2unix tool and might cause problem to other coder using linux/mac if I commited it. But there were some changes on readme.md caused by the tool which I think are not relevant to the changes I made. Please review and let me know your thoughts. Thanks

Copy link
Contributor

@morgante morgante left a comment

Choose a reason for hiding this comment

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

Linters are failing, but I will clean up separately.

@morgante morgante merged commit 92cc19f into terraform-google-modules:master May 5, 2020
@c0ffeec0der c0ffeec0der deleted the feat/secureboot branch May 6, 2020 06:51
CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…onitoring options (terraform-google-modules#506)

* Expose node pool shielded_instance_config

Co-authored-by: c0feec0der <>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants