Undefined check for apiserver_loadbalancer_domain_name in apiserver_sans#13009
Conversation
|
/ok-to-test |
|
/retest-failed |
|
Hum, select does not filter undefined? I thought I remembered it did. Did that change in Ansible core 2.18 ? Or I'm just hallucinating stuff 😆
We should also add to one the testcase an explicit loadbalancer_apiserver_localhost: false, to catch that sort of regression.
|
|
/lgtm |
|
@VannTen: once the present PR merges, I will cherry-pick it on top of DetailsIn response to this:
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-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chadswen, VannTen The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@VannTen: new pull request created: #13014 DetailsIn response to this:
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-sigs/prow repository. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Defaults
apiserver_loadbalancer_domain_nameto''inapiserver_sans, like the other nullable vars in the task, preventing undefined variable related fatal ansible errors.How to reproduce
When
loadbalancer_apiserver_localhost: falseandloadbalancer_apiserver.addressis not set (e.g. when the nginx-proxy static pod and a load balancer are not desired).Root Cause
One of the undefined checks for this variable was removed from
apiserver_sansin PR 12507 when theset_facttemplating was simplified.However, when
loadbalancer_apiserver_localhost: falseandloadbalancer_apiserver.addressis not set,apiserver_loadbalancer_domain_namedefaults toundefinedinstead of a string as of the cleanup in PR 12872 .Special notes for your reviewer:
I realize
apiserver_loadbalancer_domain_namewill be removed eventually by #12897 (which will be a great improvement IMO), but in the meantime this is blocking some of my cluster validation tests that don't provision a load balancer.Does this PR introduce a user-facing change?: