Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Quick implementation of container IP records #226

Merged
merged 16 commits into from
Aug 10, 2015
Merged

Conversation

sttts
Copy link
Contributor

@sttts sttts commented Aug 10, 2015

@sttts
Copy link
Contributor Author

sttts commented Aug 10, 2015

/cc @tsenart for early review

@sttts sttts changed the title WIP: quick implementation of container IP records Quick implementation of container IP records Aug 10, 2015
target := canonical + tail + ":" + strconv.Itoa(port.Number)

// use protocol if defined, fallback to tcp+udp
if port.Protocol != "" {
Copy link
Contributor

Choose a reason for hiding this comment

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

should we attempt to sanitize the port.Protocol label before splicing it in?

The old validation code expanded resolvers, then checked for duplicates.

This reverts commit 21c43d0.
@jdef jdef self-assigned this Aug 10, 2015
@jdef jdef added the WIP label Aug 10, 2015
@tsenart
Copy link
Contributor

tsenart commented Aug 10, 2015

@jdef: Why did you add the WIP label? Planning to do some work on it?

@jdef
Copy link
Contributor

jdef commented Aug 10, 2015

@tsenart yes, I'm going to update docs for A and SRV records

@tsenart
Copy link
Contributor

tsenart commented Aug 10, 2015

👍

HTTPOn: true,
ExternalOn: true,
RecurseOn: true,
IPSources: []string{"docker", "mesos", "host"},
Copy link
Contributor

Choose a reason for hiding this comment

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

the default should not include "docker" since Docker, by default, uses IPs that are not cluster routable

@jdef jdef added PTAL and removed WIP labels Aug 10, 2015
jdef added a commit that referenced this pull request Aug 10, 2015
Quick implementation of container IP records
@jdef jdef merged commit 0095537 into master Aug 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants