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

fix of helm installation command in doc #3333

Merged
merged 3 commits into from
Aug 24, 2023

Conversation

ashutosji
Copy link
Contributor

What type of PR is this?

Uncomment only one /kind <> line, press enter to put that in a new line, and remove leading whitespace from that line:

/kind breaking
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind hotfix
/kind release

What this PR does / Why we need it: This will fix helm installation command when we are using control TLS certificate.

Which issue(s) this PR fixes: #3323

Closes #3323

Special notes for your reviewer:

@github-actions github-actions bot added the kind/documentation Documentation for Agones label Aug 18, 2023
@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: cddd882d-8e26-4403-a45d-3f79610527c5

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3333/head:pr_3333 && git checkout pr_3333
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.35.0-dev-38c9e8a-amd64

--set agones.controller.customCertSecretPath[1].key='tls.crt',customCertSecretPath[1].path='server.crt'
--set agones.controller.customCertSecretPath[2].key='tls.key',customCertSecretPath[2].path='server.key'
--set agones.controller.allocationApiService.annotations={'cert-manager.io/inject-ca-from': 'agones-system/my-release-cert'} \
--set-string "agones.controller.customCertSecretPath[0].key=ca.crt" \
Copy link
Member

Choose a reason for hiding this comment

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

I'm actually thinking - let's remove this top section with all the --set commands, I don't think anyone would do it in practice, and it's far easier to read with the yaml file.

So let's just keep the YAML file version (unless anyone disagrees?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, changes added!

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 7d8d436a-5add-4d5b-95a1-6b62ec19725b

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3333/head:pr_3333 && git checkout pr_3333
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.35.0-dev-a571395-amd64

Copy link
Member

@markmandel markmandel left a comment

Choose a reason for hiding this comment

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

Just some small wording nits, but otherwise, looks great!

issuerRef:
name: selfsigned
kind: ClusterIssuer
EOF
```

After the certificates are generated, we will want to [inject caBundle](https://cert-manager.io/docs/concepts/ca-injector/) into controller webhook and disable controller secret creation by setting the following:
After the certificates are generated, we will want to [inject caBundle](https://cert-manager.io/docs/concepts/ca-injector/) into controller and extensions webhook and disable controller and extensions secret creation by using yaml file. This process avoids the cluter of passing too many parameters to helm install command. Please refer below `file_name.yaml` file:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: some sentence structure.

Suggested change
After the certificates are generated, we will want to [inject caBundle](https://cert-manager.io/docs/concepts/ca-injector/) into controller and extensions webhook and disable controller and extensions secret creation by using yaml file. This process avoids the cluter of passing too many parameters to helm install command. Please refer below `file_name.yaml` file:
After the certificates are generated, we will want to [inject caBundle](https://cert-manager.io/docs/concepts/ca-injector/) into the controller and extensions webhook and disable the controller and extensions secret creation through the following values.yaml file.:


```
Copy link
Member

Choose a reason for hiding this comment

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

Just for some syntax highlighting.

Suggested change
```
```yaml

disableCaBundle: true
```

After, creating yaml file use below command to install Agones:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
After, creating yaml file use below command to install Agones:
After copying the above yaml into a `values.yaml` file, use below command to install Agones:

--set agones.controller.mutatingWebhook.disableCaBundle=true \
--namespace agones-system --create-namespace \
agones/agones
helm install my-release --namespace agones-system --create-namespace --values /path/file_name.yaml agones/agones
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
helm install my-release --namespace agones-system --create-namespace --values /path/file_name.yaml agones/agones
helm install my-release --namespace agones-system --create-namespace --values values.yaml agones/agones

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for suggestions! I have updated the PR.

@agones-bot
Copy link
Collaborator

Build Succeeded 👏

Build Id: 0888032d-f26c-48d1-88e7-3435a8fa7d13

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:

  • git fetch https://github.com/googleforgames/agones.git pull/3333/head:pr_3333 && git checkout pr_3333
  • helm install agones ./install/helm/agones --namespace agones-system --set agones.image.registry=us-docker.pkg.dev/agones-images/ci --set agones.image.tag=1.35.0-dev-762c545-amd64

@google-oss-prow
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ashutosji, 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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@markmandel markmandel merged commit cb14489 into googleforgames:main Aug 24, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

helm installation is not working of Controller TLS certificate
3 participants