Skip to content
This repository was archived by the owner on Feb 24, 2020. It is now read-only.

DNS configuration improvements #3161

Merged
merged 3 commits into from
Sep 15, 2016
Merged

DNS configuration improvements #3161

merged 3 commits into from
Sep 15, 2016

Conversation

@squeed
Copy link
Contributor

@squeed squeed commented Sep 2, 2016 •
edited
Loading

  • Respect DNS results from CNI
  • Test CNI compliance with cniproxy
  • Add --dns=host mode to bind-mount the host's /etc/resolv.conf
  • Add --dns=none mode to ignore CNI DNS
  • Add --hosts-entry (IP=HOSTNAME) to tweak the pod's /etc/hosts
  • Add --hosts-entry=host to bind-mount the host's /etc/hosts

Fixes #2044, #2602, #3081

@squeed
Copy link
Contributor Author

squeed commented Sep 2, 2016

Sorry, this kind of grew out-of-hand.

s-urbaniak
s-urbaniak reviewed Sep 2, 2016
View reviewed changes
common/resolv_conf.go
}
return content

}
Copy link
Contributor

@s-urbaniak s-urbaniak Sep 2, 2016

Choose a reason for hiding this comment

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

superfluous newline

lucab
lucab reviewed Sep 2, 2016
View reviewed changes
pkg/flag/pairlist.go Outdated
// EMPTY is a small shortcut for the empty map
var EMPTY = map[string]string{}

// NewParList initializes a new pair list
Copy link
Member

@lucab lucab Sep 2, 2016

Choose a reason for hiding this comment

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

s/NewParList/NewPairList/

@squeed squeed force-pushed the dns-config-improvements branch 2 times, most recently from 68b1881 to b2572b0 Compare September 2, 2016 15:13
lucab
lucab reviewed Sep 3, 2016
View reviewed changes
stage1_fly/aci/aci-manifest.in
},
{
"name": "coreos.com/rkt/stage1/interface-version",
"value": "4"
Copy link
Member

@lucab lucab Sep 3, 2016

Choose a reason for hiding this comment

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

@alban @iaguis do you know why fly was not advertising this at all before?

Copy link
Member

@iaguis iaguis Sep 5, 2016

Choose a reason for hiding this comment

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

#2438 (comment)

Copy link
Contributor Author

@squeed squeed Sep 6, 2016

Choose a reason for hiding this comment

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

I really need to bump the version - @iaguis would it be OK if I accept --hostname but ignore it? Should I warn?

Copy link
Contributor Author

@squeed squeed Sep 15, 2016

Choose a reason for hiding this comment

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

Spoke to Iago - the only issue with bumping the version was that stage0 will no longer warn on unimplemented flags. So, I moved the warning code to stage1 and bumped the version.

@squeed squeed force-pushed the dns-config-improvements branch 2 times, most recently from 741de87 to abbfb67 Compare September 6, 2016 13:35
@squeed
Copy link
Contributor Author

squeed commented Sep 6, 2016 •
edited
Loading

TODO:

  • ask @iaguis about bumping rkt_fly's stage1 version
  • Confirm it's OK to include the CNI_VERSION environment var from @steveej

@squeed squeed force-pushed the dns-config-improvements branch from abbfb67 to 05aeec8 Compare September 9, 2016 16:50
@squeed squeed force-pushed the dns-config-improvements branch from 05aeec8 to 39e994e Compare September 13, 2016 18:03
@squeed
Copy link
Contributor Author

squeed commented Sep 13, 2016

As per an offline conversation with @iaguis, I configured rkt-fly to warn on unused parameters. It is now OK to bump the stage1 version.

No comment from @steveej about bumping the CNI version, but seeing as that is the version we vendor, and I've checked the spec, we're OK to do this.

@squeed squeed force-pushed the dns-config-improvements branch from 39e994e to 219853e Compare September 13, 2016 18:06
s-urbaniak
s-urbaniak reviewed Sep 14, 2016
View reviewed changes
networking/netinfo/netinfo.go Outdated

const filename = "net-info.json"

//A type and some structure to represent rkt's view of a *runtime*
Copy link
Contributor

@s-urbaniak s-urbaniak Sep 14, 2016

Choose a reason for hiding this comment

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

Please add a space between // and the comment.

@squeed
rkt/run: improve DNS configuration:
5aca114 
  * Respect DNS results from CNI
  * Add --dns=host mode to bind-mount the host's /etc/resolv.conf
  * Add --dns=none mode to ignore CNI DNS
  * Add --hosts-entry (IP=HOSTNAME) to tweak the pod's /etc/hosts
  * Add --hosts-entry=host to bind-mount the host's /etc/hosts
@squeed squeed force-pushed the dns-config-improvements branch from 219853e to 5aca114 Compare September 14, 2016 11:34
@squeed
Copy link
Contributor Author

squeed commented Sep 14, 2016

Small bug broke rkt fly tests - fixed. This is ready for final review.

@s-urbaniak s-urbaniak added this to the v1.15.0 milestone Sep 15, 2016
@s-urbaniak
Copy link
Contributor

s-urbaniak commented Sep 15, 2016

LGTM

@s-urbaniak s-urbaniak merged commit f02805b into rkt:master Sep 15, 2016
@danielmotaleite danielmotaleite mentioned this pull request Feb 7, 2018
Open
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@lucab lucab lucab left review comments

@s-urbaniak s-urbaniak s-urbaniak left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/networking reviewed/lgtm

Projects

None yet

Milestone

v1.15.0

Development

Successfully merging this pull request may close these issues.

6 participants

@squeed @s-urbaniak @lucab @iaguis @jellonek @jonboulle