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
[occm] Fixed the typo in the load balancing section in the README #2232
Conversation
Hi @armagankaratosun. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
sorry, i made a mistake while rebasing my previous pull request #2183 and needed to reopen this. |
seems there are similiar 2 commits in the PR and ,is it because of git rebase? |
I made a typo once, then fixed it again. |
/retest-required |
hello @jichenjc, I don't understand why the tests are failing ( in my simple understanding, the linting is failing not because of me or my change but maybe I am doing something wrong). In my defense, this was a simple PR where I fix only the README (this doesn't mean that this is not important though). Can you please help me to identify what is wrong? |
These were broken until recently. Might pass now. /test openstack-cloud-csi-manila-e2e-test However, you are right that we don't need to run them against a README change. There's stuff we can configure in prow to do this, but we need to be careful with it. |
right , previously there are changes that make some file change didn't run CI.. so need to be careful now I am confused why it reports issue against helm chart, maybe you can try rebase? |
ok, trying to rebase again |
fixed the typo on loadbalancer config
731c50b
to
e621b72
Compare
/lgtm |
/approve |
An obvious fix. /lgtm |
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.
/approve
thank you
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kayrus, zetaab 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 |
@armagankaratosun I think that lint check means you need to bump a version number in the helm charts, but:
I think @zetaab has knowledge of the deep magic? |
Since this README.md file was modified under the chart dir, it's required to manually bump the minor chart version before the merge. See https://github.com/kubernetes/cloud-provider-openstack/pull/2254/files for example. |
Oh, now it is clear, thanks! Then, I will bump up the version. TBH, now that I understand what the issue is, it started to make perfect sense 😄 |
/lgtm |
…bernetes#2232) * fixed the typo on loadbalancer config fixed the typo on loadbalancer config * bumped the openstack-cloud-controller-manager chart version
[occm] Fixed the typo in the load balancing section in the README
All the currently maintained binaries are:
What this PR does / why we need it:
If you set the variable with a lowercase "b", the template will not catch that and the secret that is created via the helm chart will lack the [LoadBalancer] section. the correct value should be "loadBalancer", instead of "loadbalancer", as you can see from the values.yaml.
Release note: