Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Switch from command line flags to config file based options #77

Merged
merged 2 commits into from
Mar 1, 2022

Conversation

shuheiktgw
Copy link
Contributor

@shuheiktgw shuheiktgw commented Feb 24, 2022

What this PR does / Why we need it

Switched from command line flags to config file-based options.
https://book.kubebuilder.io/component-config-tutorial/tutorial.html

Which issue(s) this PR fixes

None

Steps to test the update

make kind-cluster-reset

export IMG=mercari/spanner-autoscaler:local
make docker-build && make kind-load-docker-image

kubectl apply -f https://github.com/jetstack/cert-manager/releases/download/v1.6.1/cert-manager.yaml
make deploy

@shuheiktgw shuheiktgw changed the title [WIP] Switch from command line flags to config file based options Switch from command line flags to config file based options Feb 26, 2022
@shuheiktgw shuheiktgw marked this pull request as ready for review February 26, 2022 02:12
@shuheiktgw shuheiktgw requested a review from a team as a code owner February 26, 2022 02:12
@@ -18,9 +18,4 @@ spec:
- "--v=10"
ports:
- containerPort: 8443
name: https
- name: manager
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This conflicts with manager_config_patch.yaml so I removed it

Comment on lines -24 to -26
- "--health-probe-bind-address=:8081"
- "--metrics-bind-address=127.0.0.1:8080"
- "--leader-elect"
Copy link
Contributor

Choose a reason for hiding this comment

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

$ kustomize build config/default

...
---
apiVersion: v1
data:
  controller_manager_config.yaml: |
    apiVersion: controller-runtime.sigs.k8s.io/v1alpha1
    kind: ControllerManagerConfig
    health:
      healthProbeBindAddress: :8081
    metrics:
      bindAddress: 127.0.0.1:8080
    webhook:
      port: 9443
    leaderElection:
      leaderElect: true
      resourceName: 54b82eb3.mercari.com
kind: ConfigMap
metadata:
  name: spanner-autoscaler-manager-config
  namespace: spanner-autoscaler

Copy link
Contributor

@tkuchiki tkuchiki left a comment

Choose a reason for hiding this comment

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

LGTM

@tkuchiki
Copy link
Contributor

tkuchiki commented Mar 1, 2022

After reviewing @rustycl0ck, we'll merge this PR.

Copy link
Contributor

@rustycl0ck rustycl0ck left a comment

Choose a reason for hiding this comment

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

LGTM

@rustycl0ck rustycl0ck merged commit 026cde9 into mercari:master Mar 1, 2022
@rustycl0ck rustycl0ck added the enhancement New feature or request label Mar 14, 2022
rustycl0ck pushed a commit to rustycl0ck/spanner-autoscaler that referenced this pull request Apr 20, 2022
* Switch from command line flags to config file based options

* Update config/default
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants