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

add ability for custom flags #1232

Merged
merged 5 commits into from
Apr 17, 2017
Merged

add ability for custom flags #1232

merged 5 commits into from
Apr 17, 2017

Conversation

rsmitty
Copy link
Contributor

@rsmitty rsmitty commented Apr 14, 2017

This addresses #899, as well as possibly addressing #1193. Would provide a good way to add flags that are more specific to a cluster's environment and size, but not useful for everyone.

@rsmitty
Copy link
Contributor Author

rsmitty commented Apr 14, 2017

ci check this

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 14, 2017
@@ -81,6 +81,9 @@ spec:
{% if kube_api_anonymous_auth is defined and kube_version | version_compare('v1.5', '>=') %}
- --anonymous-auth={{ kube_api_anonymous_auth }}
{% endif %}
{% for flag in apiserver_custom_flags %}
Copy link
Contributor

Choose a reason for hiding this comment

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

There are so few lists in params that I am worried about users trying to use this with a plain string and then it will be split in to a list of chars. This is an alternative that will handle both string or list:

{% if apiserver_custom_flags is string %}
    - {{ apiserver_custom_flags }}
{% else %}
  {% for flag in apiserver_custom_flags %}
    - {{ flag }}
  {% endfor %}
{% endif %}

What are your thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, can't hurt to safeguard around that. Will update.

@rsmitty
Copy link
Contributor Author

rsmitty commented Apr 17, 2017

Alright, updated and ready to roll.

@@ -81,6 +81,13 @@ spec:
{% if kube_api_anonymous_auth is defined and kube_version | version_compare('v1.5', '>=') %}
- --anonymous-auth={{ kube_api_anonymous_auth }}
{% endif %}
{% if apiserver_custom_flags is string %}
- {{ apiserver_custom_flags }}
{% else % }
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax error because of "% }" - remove the space

@@ -46,6 +46,13 @@ spec:
- --configure-cloud-routes=true
- --cluster-cidr={{ kube_pods_subnet }}
{% endif %}
{% if controller_mgr_custom_flags is string %}
- {{ controller_mgr_custom_flags }}
{% else % }
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax error because of "% }" - remove the space

@@ -27,6 +27,13 @@ spec:
- --leader-elect=true
- --master={{ kube_apiserver_endpoint }}
- --v={{ kube_log_level }}
{% if scheduler_custom_flags is string %}
- {{ scheduler_custom_flags }}
{% else % }
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax error because of "% }" - remove the space

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants