-
Notifications
You must be signed in to change notification settings - Fork 16
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
don't use obscure env variables and use flags #44
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea 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 |
/assign @danwinship |
install-anp.yaml
Outdated
@@ -80,7 +80,7 @@ spec: | |||
- name: kube-network-policies | |||
image: registry.k8s.io/networking/kube-network-policies:v0.3.0 | |||
command: ["/bin/netpol"] | |||
args: ["-v=2", "-admin-network-policy=true", "-baseline-admin-network-policy=true"] | |||
args: ["-v=2", "-admin-network-policy=true", "-baseline-admin-network-policy=true","--hostname-override=${MY_NODE_NAME}"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the main problem because I added the env variables is because I was not sure you can do this, of using the env variable as the flag value and I was too lazy ... now that you mentioned I can see this is exactly how most people do https://grep.app/search?q=hostname-override%3D%24&filter[lang][0]=YAML
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ups , something is wrong
2024-06-26T10:58:00.54696576Z stderr F I0626 10:58:00.546903 1 controller.go:89] Creating controller: networkpolicy.Config{FailOpen:false, AdminNetworkPolicy:false, BaselineAdminNetworkPolicy:false, QueueID:100, NodeName:"${my_node_name}"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8f1377e
to
0e8501c
Compare
/lgtm |
New changes are detected. LGTM label has been removed. |
/hold cancel fixed the env variable problem and added the error to the log, once CI passes we should be good to go and I fix existing release |
Let's do the things the right way
Fixes: #43