Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Packet worker/controller: improve wait-for-dns output #735

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

margamanterola
Copy link
Contributor

dig returns 0 even if the domain passed doesn't exist. So the script as is was printing Failed with exit code 0: when there was a nameserver configuration error, which was not a useful error message:

● wait-for-dns.service - Wait for DNS entries
   Loaded: loaded (/etc/systemd/system/wait-for-dns.service; enabled; vendor preset: enabled)
   Active: activating (start) since Mon 2020-07-27 14:20:17 UTC; 9min ago
 Main PID: 7128 (sh)
    Tasks: 3 (limit: 32767)
   Memory: 5.3M
   CGroup: /system.slice/wait-for-dns.service
           ├─ 7128 /bin/sh -c while ! /usr/bin/grep ^[^#[:space:]] /etc/resolv.conf > /dev/null; do sleep 1; done; /o>
           ├─ 7130 /bin/bash /opt/wait-for-dns infra.flatcar-linux.net monitoring-private 3600
           └─17621 sleep 1

Jul 27 14:29:39 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:40 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:42 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:44 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:46 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:48 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:49 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:51 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:53 monitoring-controller-0 sh[7128]: Failed with exit code 0:
Jul 27 14:29:55 monitoring-controller-0 sh[7128]: Failed with exit code 0:

Instead, modify these scripts to print the actual error (that the nameservers weren't found) when the output is empty.

@margamanterola margamanterola requested a review from ipochi July 27, 2020 15:15
Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @marga-kinvolk. Please run make update-assets and commit generated code together with your changes, to make the CI pass.

I also pointed one suggestion in the comment.

@margamanterola margamanterola force-pushed the marga-kinvolk/error-message branch from 59801f7 to 07e743b Compare July 27, 2020 15:24
Copy link
Member

@ipochi ipochi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @marga-kinvolk

+1 for make update-assets , even running make should suffice. Generated files is pkg/assets/generated_assets.go

dig returns 0 even if the domain passed doesn't exist. So the script as
is was printing `Failed with exit code 0:` which was not a useful error
message.

Instead, modify these scripts to print the actual error (that the
nameservers weren't found) when the output is empty.
@margamanterola margamanterola force-pushed the marga-kinvolk/error-message branch from 07e743b to de44146 Compare July 27, 2020 16:13
@margamanterola
Copy link
Contributor Author

margamanterola commented Jul 27, 2020

Sorry, I missed the callout to make update-assets before. Done now.

Copy link
Member

@invidian invidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@ipochi ipochi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@invidian invidian merged commit 1afc56c into master Jul 28, 2020
@invidian invidian deleted the marga-kinvolk/error-message branch July 28, 2020 08:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants