[Issue-12242]-Verify whether main_access_ip is present inside the hostvars dictionary when using the --limit option

[kartsank]

…ionary

What type of PR is this?
/kind bug

What this PR does / why we need it:
Verify whether main_access_ip is present inside the hostvars dictionary while creating host list from inventory

Which issue(s) this PR fixes:
Fixes #12242

Special notes for your reviewer:
NONE
Does this PR introduce a user-facing change?:
NONE

NONE

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: kartsank
Once this PR has been reviewed and has the lgtm label, please assign vannten for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-ci-robot]

Hi @kartsank. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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-sigs/prow repository.

[ant31]

/lgtm

[chadswen]

/ok-to-test

[VannTen]

This is not the correct fix, this simply bypass the problem.
the variable should be defined, => this is why you need to run the facts playbook before scaling up.

[ant31]

@VannTen Do you have it working with --limit ? I m not. so maybe there are some command / configuration with ansible ?

In all cases, it's the only blocking point for using --limit,
we should be allowed to scale-up without having to fact fetch 100s of nodes before.
It should only requires one control-plane, and the new nodes

[VannTen]

Don't disagree that we need to scrap the whole etc/hosts stuff if we can, but in that case there is no reason to patch it up, better to cleanly remove it than still doing it but with missing stuff. I haven't tested limit recently, it's the start of the busy period here.

[kartsank]

Don't disagree that we need to scrap the whole etc/hosts stuff if we can, but in that case there is no reason to patch it up, better to cleanly remove it than still doing it but with missing stuff. I haven't tested limit recently, it's the start of the busy period here.

Just back from vacation. Thanks for the comments.

This is not new fix I just add this if check from previous version of this file. see the file roles/kubernetes/preinstall/tasks/0090-etchosts.yml on line 6 on PR.

https://github.com/kubernetes-sigs/kubespray/pull/11953/files#diff-9c190d5e9ce1325bf9227a5326358978dce36b74bfc6baeda861eb2ccafe4239L6

{% if 'access_ip' in hostvars[item] or 'ip' in hostvars[item] or 'ansible_default_ipv4' in hostvars[item] -%} {%- if ('ansible_hostname' in hostvars[item] and item != hostvars[item]['ansible_hostname']) %} {{ hostvars[item]['ansible_hostname'] }}.{{ dns_domain }} {{ hostvars[item]['ansible_hostname'] }} {% else %} {{ item }}.{{ dns_domain }} {{ item }} {% endif %}

Agree if we run facts.yml it will gather the vars or I can use skip tags ( --skip-tags=etchosts) to skip the etchosts update.

but if we are running ansible on docker container. I can't keep facts cache on my next execution.