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

fqdn_ip4 and fqdn_ip6 are empty on 2016.3+ #34129

Closed
onorua opened this issue Jun 20, 2016 · 13 comments
Closed

fqdn_ip4 and fqdn_ip6 are empty on 2016.3+ #34129

onorua opened this issue Jun 20, 2016 · 13 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt fixed-pls-verify fix is linked, bug author to confirm fix Grains P3 Priority 3 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around ZRELEASED - 2016.3.2
Milestone

Comments

@onorua
Copy link
Contributor

onorua commented Jun 20, 2016

Description of Issue/Question

I've discovered really strange behaviour after upgrade, fqdn_ip4 and fqdn_ip6 are empty.
According to documentation it is supported and no word about deprecation

Steps to Reproduce Issue

# salt-call --versions-report
                  Salt: 2015.5.3
                Python: 2.7.10 (default, Oct 14 2015, 16:09:02)
                Jinja2: 2.8
              M2Crypto: 0.21.1
        msgpack-python: 0.4.2
          msgpack-pure: Not Installed
              pycrypto: 2.6.1
               libnacl: Not Installed
                PyYAML: 3.11
                 ioflo: Not Installed
                 PyZMQ: 14.4.1
                  RAET: Not Installed
                   ZMQ: 4.0.5
                  Mako: 1.0.2
               Tornado: 4.2.1
 Debian source package: 2015.5.3+ds-1

# salt-call grains.item fqdn_ip4 
local:
    ----------
    fqdn_ip4:
        - 172.16.236.1

but as soon as I upgrade to version 2016.3.0 or 1, I've got following result:

# salt-call --versions-report
Salt Version:
           Salt: 2016.3.1

Dependency Versions:
           cffi: 1.1.2
       cherrypy: Not Installed
       dateutil: 2.2
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: 0.22.2
        libnacl: Not Installed
       M2Crypto: 0.21.1
           Mako: 1.0.2
   msgpack-pure: Not Installed
 msgpack-python: 0.4.2
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
         pygit2: 0.22.0
         Python: 2.7.10 (default, Oct 14 2015, 16:09:02)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.4.1
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

System Versions:
           dist: Ubuntu 15.10 wily
        machine: x86_64
        release: 4.2.0-27-generic
         system: Linux
        version: Ubuntu 15.10 wily
# salt-call grains.item fqdn_ip4
local:
    ----------
    fqdn_ip4:

I believe this is a bug, but let me know if I'm missing some configuration here.

@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 20, 2016

@onorua I am able to replicate this but only on ubuntu15 not centos7. HOw are you installing salt on ubuntu15?

For record I installed it using the bootstrap script as a git install v2016.3.1

@Ch3LL Ch3LL added Bug broken, incorrect, or confusing behavior P3 Priority 3 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around Core relates to code central or existential to Salt Grains labels Jun 20, 2016
@Ch3LL Ch3LL added this to the Approved milestone Jun 20, 2016
@onorua
Copy link
Contributor Author

onorua commented Jun 21, 2016

I'm installing it using deb repository for trusty, and apt-get install salt-* all the steps are done from salt state. I can copy paste it here, but I believe it is not needed as you've already reproduced the issue.
We do not use Centos or any RPM based systems, I can't comment whether it is working there or not :(

@Ch3LL
Copy link
Contributor

Ch3LL commented Jun 21, 2016

@onorua thanks for the additional information. Yeah we don't need the additional state file. Thanks we will get this fixed up.

@MartinEmrich
Copy link

Same here: (CentOS 7):

[root@ip-10-13-10-85 salt]# salt-call grains.get fqdn_ip4
local:
[root@ip-10-13-10-85 salt]# salt-minion --version
salt-minion 2016.3.1 (Boron)

@meggiebot meggiebot modified the milestones: C 7, Approved Jun 23, 2016
@meggiebot meggiebot modified the milestones: C 8, C 7 Jun 23, 2016
@rallytime
Copy link
Contributor

rallytime commented Jun 23, 2016

@onorua Does the domain grain have anything in it on these machines? The fqdn_* grains were optimized in #29346 to only populate that grain data when a domain is present to help speed up grain loading by minimizing DNS resolution calls.

Therefore, I believe this grain to no longer be populated is intentional (if domain is indeed empty for you). Thoughts?

@rallytime
Copy link
Contributor

Also, it is worth mentioning that while I was able to reproduce what you're seeing between the 2015.8 and 2016.3 branches, I was only able to do so on minions where the domain grain was empty.

@rallytime rallytime added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jun 23, 2016
@onorua
Copy link
Contributor Author

onorua commented Jun 24, 2016

Indeed, when I populate domain - fqdn_ipv4 populates:

    domain:
        k8s.otlabs.fr
    fqdn:
        ip-172-16-236-25.k8s.otlabs.fr
    fqdn_ip4:
        - 172.16.236.25
    fqdn_ip6:

one thing bothers me though... this dependency never stated in documentation, on the systems without domain I can see following:

    domain:
    fqdn:
        ip-172-16-236-1
    fqdn_ip4:
    fqdn_ip6:

but I can ping fqdn no problems:

ping -c 1 ip-172-16-236-1
PING ip-172-16-236-1 (172.16.236.1) 56(84) bytes of data.
64 bytes from ip-172-16-236-1 (172.16.236.1): icmp_seq=1 ttl=64 time=0.057 ms

What are the requirements to get domain populated automatically? I could not find this in documentation :(

@rallytime
Copy link
Contributor

@onorua Right - thanks for pointing that out. It looks like #29346 has caused a genuine regression here.

@adelcast I would like to loop you in here. If there isn't a way to fully restore this behavior, we'll have to revert the change you made in #29346. If it's possible, I'd like to keep the core grains loading light and restore this behavior, but I am not seeing a way to do that currently which the way the code sits right now.

@adelcast
Copy link
Contributor

Looks like the check on fqdn_ip4 and fqdn_ip6 to skip the socket.getaddrinfo call is incomplete since there seems to be cases where the grains['domain'] is empty, but the sockert.getaddrinfo call succeeds, as @onorua pointed out.

I ran a few test on my system (Linux 4.1.15, previously I was using 3.15) where I reintroduced the old behavior on fqdn_ip4 and fqdn_ip6. This time socket.getaddrinfo failed quickly....so it's likely that the network stack got better (fails faster).

So, I agree, I think we can safely partially revert #29346 (keep the optimization on hostname()). Thanks for the detective work and apologies for the troubles.

@rallytime
Copy link
Contributor

@adelcast Thank you for looking into this and confirming. That makes sense. I can grab the partial-revert on #29346.

@rallytime
Copy link
Contributor

rallytime commented Jun 24, 2016

@adelcast and @onorua Please see the change in #34284. Thank you all for reporting, commenting, and investigating!

@rallytime rallytime added fixed-pls-verify fix is linked, bug author to confirm fix and removed Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged labels Jun 24, 2016
@adelcast
Copy link
Contributor

looks good to me, thanks for taking care of this!

@onorua
Copy link
Contributor Author

onorua commented Jun 26, 2016

    domain:
    fqdn:
        ip-172-16-236-1
    fqdn_ip4:
        - 172.16.236.1
    fqdn_ip6:

I can confirm that fqdn_ipv4 is populated with proposed fix, so I'm going to close the ticket now, Thanks for the prompt fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt fixed-pls-verify fix is linked, bug author to confirm fix Grains P3 Priority 3 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around ZRELEASED - 2016.3.2
Projects
None yet
Development

No branches or pull requests

6 participants