-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
grains/core: ignore HOST_NOT_FOUND errno in fqdns() #51706
Conversation
fee6f42
to
3c05bdc
Compare
Is ready for review. @isbm what do you think? |
* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) saltstack/salt#50125 * Add root parameter to useradd, shadow and groupadd saltstack/salt#50175 * cmd: Add root parameter for wait and run states saltstack/salt#50302 * systemd: add optional root parameter saltstack/salt#50380 * Add new chroot module https://github.com/openSUSE/salt/pull/50418 * Add new module freezer saltstack/salt#50452 * btrfs: add all subvolume commands saltstack/salt#50541 * btrfs: add new btrfs state saltstack/salt#50635 * zypper: demote log from error to warning saltstack/salt#50671 * blkid: add search by token saltstack/salt#50706 * mount: add fstab_{present,absent} states saltstack/salt#50725 * btrfs: add option to not set subvolumes as default saltstack/salt#50801 * Add disk_set and disk_toggle functions, and update valid partition flags saltstack/salt#50834 * disk: support setting FAT size for format_ saltstack/salt#51074 * parted: fix set_ valid flags comment. saltstack/salt#51704 * grains/core: ignore HOST_NOT_FOUND errno in fqdns() saltstack/salt#51706 * cmdmod: add 'binds' parameter in run_chroot saltstack/salt#51871 * mount: fix extra -t parameter saltstack/salt#51905 * lvm: be quiet when a pv, lv or vg is not expected saltstack/salt#51929 * linux_lvm: clean error in pvcreate and pvremove saltstack/salt#51954 * blockdev: hide blkid errors when are expected saltstack/salt#51956 * partially unify public functions signature for pkg and lowpkg saltstack/salt#51973 * extmods: add utils directories in sys.path saltstack/salt#52001
e475011
to
b9c88e7
Compare
60c42f4
to
19669eb
Compare
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.
lgtm
ca362cb
to
84f1d6d
Compare
@aplanas would it be reasonable to use the "warn once" method that we have elsewhere in the code, or does it make more sense to always ignore it? |
Oh I like the idea. Makes sense for me. I will check how this was done before. |
@waynew maybe I will need a bit of help here. The only place that I found where there is some kind of repetition filter for warnings is in __init__.py, but this is related with exceptions, and not with logs. |
* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) saltstack/salt#50125 * Add root parameter to useradd, shadow and groupadd saltstack/salt#50175 * cmd: Add root parameter for wait and run states saltstack/salt#50302 * systemd: add optional root parameter saltstack/salt#50380 * Add new chroot module https://github.com/openSUSE/salt/pull/50418 * Add new module freezer saltstack/salt#50452 * btrfs: add all subvolume commands saltstack/salt#50541 * btrfs: add new btrfs state saltstack/salt#50635 * zypper: demote log from error to warning saltstack/salt#50671 * blkid: add search by token saltstack/salt#50706 * mount: add fstab_{present,absent} states saltstack/salt#50725 * btrfs: add option to not set subvolumes as default saltstack/salt#50801 * Add disk_set and disk_toggle functions, and update valid partition flags saltstack/salt#50834 * disk: support setting FAT size for format_ saltstack/salt#51074 * parted: fix set_ valid flags comment. saltstack/salt#51704 * grains/core: ignore HOST_NOT_FOUND errno in fqdns() saltstack/salt#51706 * cmdmod: add 'binds' parameter in run_chroot saltstack/salt#51871 * mount: fix extra -t parameter saltstack/salt#51905 * lvm: be quiet when a pv, lv or vg is not expected saltstack/salt#51929 * linux_lvm: clean error in pvcreate and pvremove saltstack/salt#51954 * blockdev: hide blkid errors when are expected saltstack/salt#51956 * partially unify public functions signature for pkg and lowpkg saltstack/salt#51973 * extmods: add utils directories in sys.path saltstack/salt#52001
this is potentially a bug, can we tag it for Neon? |
@aplanas I'll bring it up with @saltstack/team-core, but my feelings are that it should get in with the Neon release. |
The clint complain is about this:
As I do not touch this piece of code at all, can we review the patch? This is taking too long IMHO. |
Hey @aplanas sorry for the delay. https://github.com/saltstack/salt/blob/216e3a7f9991ad3f1650841a744e7eb2c60a8a1f/salt/grains/core.py#L44 Looks like we should just tell lint to ignore that line, apparently we're already aware of it - especially if we want to put this change into neon as well. If we're just going into the Sodium release we should be able to just remove the entire other block of code. Hm. Actually now that I look closer - maybe we should just rely on distro in general 🤔 Though I'm not actually seeing it in requirements anywhere 😕 |
Edit: I just saw a disable=W1505 in the new master, I will rebase then! But I still like the idea. |
Rebased into the new master (from develop), any chance now? |
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.
Hey @aplanas this is looking great (and, apologies for the lengthy process on this one 😞 )
Would you be able to write a test for this? I'm hoping that it shouldn't be too much effort and we have something already testing around this area...
@waynew I added a test. Can you re-review? |
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.
🚀 Sweet action!
I know we currently have some issues around our Mac testing infrastructure that we're working on, but otherwise this should be ready to go. I did notice some Centos failures that I need to check out before we merge this.
Ah. It was a test from an unrelated thing (we do still need to fix that, so we'll probably need to do one more rebase 🤞 after we fix the mac/other issue elsewhere) |
We have 49(!) checks in place, but they are still randomly failing, making very hard to differentiate between an useful failing report, from noise. Can we have a more strict policy in place, where we trade off a super big number of checks with reliable ones? For example, if ci/py2/macosxmojave is broken, lets disable it with urgency before re-adding it once that is fixed. |
ci/py3/debian10 fail seems unrelated:
|
9d1797a
to
0dd3798
Compare
This is as green as I can get. The current fails, IIUC, are timeouts. I am not able to diagnose the reason of the VM timeout, tho. |
624e834
to
6d25b32
Compare
Also ignore NO_DATA errors, as this imply an empty field Fixes saltstack#52788
What does this PR do?
We ignore also HOST_NOT_FOUND errno (actually h_errno) when trying to recover the fqdn.
Fixes #52788