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

Make nameserver overridable for provider #180

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kmpm
Copy link

@kmpm kmpm commented Jun 1, 2021

Adds a flag to the provider command that allows to override the default
nameserver in for example the systemd unit file for faasd-provider.

Description

Added a single string flag to cmd/provider.go called nameserver.
That flag has a default value of 8.8.8.8 and will get used when resolve.conf created later in the source.

Motivation and Context

  • I have raised an issue to propose this change this is required

Will probably solve #174 and #176.
Edit: made my own issue #181

How Has This Been Tested?

I ran the test provided in the code.

Built a version of it and deployed to my raspberry pi instance of faasd.
Then I modified the unit file /usr/lib/systemd/system/faasd-provider.service and restarted everything.

[Unit]
Description=faasd-provider

[Service]
MemoryLimit=500M
Environment="secret_mount_path=/var/lib/faasd/secrets"
Environment="basic_auth=true"
ExecStart=/usr/local/bin/faasd provider --nameserver 172.16.10.252
Restart=on-failure
RestartSec=10s
WorkingDirectory=/var/lib/faasd-provider

[Install]
WantedBy=multi-user.target

Then I deployed a function that looked up a database server using a hostname that is only resolvable from the defined internal DNS and everything worked.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

Commits:

  • I've read the CONTRIBUTION guide
  • My commit message has a body and describe how this was tested and why it is required.
  • I have signed-off my commits with git commit -s for the Developer Certificate of Origin (DCO)

Code:

  • My code follows the code style of this project.
  • I have added tests to cover my changes.

Docs:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Adds a flag to the provider command that allows to override the default
nameserver in for example the systemd unit file for faasd-provider.

Signed-off-by: Peter Magnusson <me@kmpm.se>
@Shikachuu
Copy link
Contributor

I think if you are already doing a remake for the resolve.conf for the sake of stability we should include a fallback nameserver.
What do you think about this @kmpm , @alexellis ?

@@ -28,6 +28,7 @@ func makeProviderCmd() *cobra.Command {
}

command.Flags().String("pull-policy", "Always", `Set to "Always" to force a pull of images upon deployment, or "IfNotPresent" to try to use a cached image.`)
command.Flags().String("nameserver", "8.8.8.8", `Set to the DNS server you want to use for resolving DNS queries`)
Copy link
Member

Choose a reason for hiding this comment

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

I would expect this to be a string array so that multiple addresses can be given, it seems standard to see 8.8.8.8 and 8.8.8.4 or the equivalent set of Cloudflare IPs.

@alexellis
Copy link
Member

Commented @Shikachuu

Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

What's your thoughts on the suggestion being made?

@kmpm
Copy link
Author

kmpm commented Nov 4, 2021

The suggestions are great. Will probably get to it this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants