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

GKE 101: added guest_accelerator feature and tests #157

Conversation

alexkonkin
Copy link
Contributor

No description provided.

Copy link
Contributor

@aaron-lane aaron-lane left a comment

Choose a reason for hiding this comment

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

The changes to cluster_regional.tf and cluster_zonal.tf need to be moved to the equivalent files under the autogen directory. Then, make generate and make generate_docs need to be executed.

There is a test failure which must be addressed:

https://concourse.infra.cft.tips/builds/4176#L5cf138fb:973

@alexkonkin
Copy link
Contributor Author

The changes to cluster_regional.tf and cluster_zonal.tf need to be moved to the equivalent files under the autogen directory. Then, make generate and make generate_docs need to be executed.

There is a test failure which must be addressed:

https://concourse.infra.cft.tips/builds/4176#L5cf138fb:973

Thank you for your review. Done.

@alexkonkin
Copy link
Contributor Author

I have one suggestion regarding the make.sh file. This file has check_generate / check_generate_docs functions. Both functions use --exit-code to detect if changes are present in the source code.
However the same return code comes to the end of the function and then to the target
in the Makefile. As the result the first of these functions makes the execution of the Makefile to fail.
My modification sets the rc (return code) back to 0 which allows all sub targets in the make target to finish.

@aaron-lane aaron-lane added the enhancement New feature or request label Jun 3, 2019
Copy link
Contributor

@aaron-lane aaron-lane left a comment

Choose a reason for hiding this comment

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

The changes to test/make.sh must be reverted; the script is working as expected. The reason why the check is failing is because the documentation needs to be regenerated with make generate_docs, as evidenced by the current removal of the Inputs and Outputs sections from the README.md.

The failure in the deploy service suite can be fixed by adding the following content to .kitchen.yml at the end of the deploy_service suite block after line 43:

    lifecycle:
      pre_verify:
        - sleep 10

Additionally, this branch needs to be rebased against master.

@alexkonkin alexkonkin force-pushed the feature/101_add_guest_accelerator branch from e490ebe to 6086f62 Compare June 3, 2019 16:05
@aaron-lane
Copy link
Contributor

aaron-lane commented Jun 3, 2019

I have tested this locally; the CI failures are unrelated.

@dkozlov
Copy link

dkozlov commented Jun 5, 2019

guest_accelerator forces new resource.

      node_config.0.guest_accelerator.0.count:         "" => "0" (forces new resource)

@alexkonkin please look at my commit which was made 3 months ago: dkozlov@e6952a4 it works.

@aaron-lane
Copy link
Contributor

@dkozlov thank you for catching that. The feature has not been released yet. Can you please open a pull request with your alternative implementation?

CPL-markus pushed a commit to WALTER-GROUP/terraform-google-kubernetes-engine that referenced this pull request Jul 15, 2024
…re/101_add_guest_accelerator

GKE 101: added guest_accelerator feature and tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants