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

Added logic to read NetworkInfo from /state.json. #266

Merged
merged 6 commits into from
Sep 17, 2015
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions records/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,22 @@ type Label struct {

// Status holds a task status as defined in the /state.json Mesos HTTP endpoint.
type Status struct {
Timestamp float64 `json:"timestamp"`
State string `json:"state"`
Labels []Label `json:"labels,omitempty"`
Timestamp float64 `json:"timestamp"`
State string `json:"state"`
Labels []Label `json:"labels,omitempty"`
ContainerStatus ContainerStatus `json:"container_status,omitempty"`
}

// ContainerStatus holds container metadata as defined in the /state.json
// Mesos HTTP endpoint.
type ContainerStatus struct {
NetworkInfos []NetworkInfo `json:"network_infos,omitempty"`
}

// NetworkInfo holds a network address resolution as defined in the
// /state.json Mesos HTTP endpoint.
type NetworkInfo struct {
IPAddress string `json:"ip_address,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we know if the IPAddress is v4 or v6? Are all clients supposed to try parsing one and then fallback?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly. Using a string here was seen as a better alternative than trying to come up with a clever packed representation.

On Sep 17, 2015, at 01:39, Tomás Senart [email protected] wrote:

In records/state/state.go:

  • Timestamp float64 json:"timestamp"
  • State string json:"state"
  • Labels []Label json:"labels,omitempty"
  • ContainerStatus ContainerStatus json:"container_status,omitempty"
    +}

+// ContainerStatus holds container metadata as defined in the /state.json
+// Mesos HTTP endpoint.
+type ContainerStatus struct {

  • NetworkInfos []NetworkInfo json:"network_infos,omitempty"
    +}

+// NetworkInfo holds a network address resolution as defined in the
+// /state.json Mesos HTTP endpoint.
+type NetworkInfo struct {

  • IPAddress string json:"ip_address,omitempty"
    How do we know if the IPAddress is v4 or v6? Are all clients supposed to try parsing one and then fallback?


Reply to this email directly or view it on GitHub.

}

// Task holds a task as defined in the /state.json Mesos HTTP endpoint.
Expand Down Expand Up @@ -103,7 +116,18 @@ func (t *Task) containerIP(src string) string {
continue
}

// find the latest docker-inspect label
// first try to extract the address from the NetworkInfo structure
if len(status.ContainerStatus.NetworkInfos) > 0 {
// TODO(CD): handle multiple NetworkInfo objects
// TODO(CD): only create A records if the address is IPv4
// TODO(CD): create AAAA records if the address is IPv6
latestContainerIP = status.ContainerStatus.NetworkInfos[0].IPAddress
Copy link
Contributor

Choose a reason for hiding this comment

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

may be interesting to filter on label here at some point in the future when there are multiple IPs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not even sure what Mesos-DNS should do when there are multiple addreses assigned. I think we'd need conventions for handling additional metadata to give us clues (e.g. labels on NetworkInfo, etc.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, it's not clear to me either how mesos-dns should respond in
situations like that. It also just occurred to me that it might be nice to
include mention of this new behavior somewhere in the mesos-dns docs pages.

On Wed, Sep 16, 2015 at 7:15 PM, Connor Doyle [email protected]
wrote:

In records/state/state.go
#266 (comment):

@@ -103,7 +116,18 @@ func (t *Task) containerIP(src string) string {
continue
}

  •   // find the latest docker-inspect label
    
  •   // first try to extract the address from the NetworkInfo structure
    
  •   if len(status.ContainerStatus.NetworkInfos) > 0 {
    
  •       // TODO(CD): handle multiple NetworkInfo objects
    
  •       // TODO(CD): only create A records if the address is IPv4
    
  •       // TODO(CD): create AAAA records if the address is IPv6
    
  •       latestContainerIP = status.ContainerStatus.NetworkInfos[0].IPAddress
    

I'm not even sure what Mesos-DNS should do when there are multiple
addreses assigned. I think we'd need conventions for handling additional
metadata to give us clues (e.g. labels on NetworkInfo, etc.)


Reply to this email directly or view it on GitHub
https://github.com/mesosphere/mesos-dns/pull/266/files#r39696900.

latestTimestamp = status.Timestamp
continue // Skip label inspection step below for this element
}

// next, fall back to the docker-inspect label
// TODO(CD): deprecate and then remove this address discovery method
for _, label := range status.Labels {
if label.Key == ipLabel {
latestContainerIP = label.Value
Expand Down
60 changes: 60 additions & 0 deletions records/state/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,63 @@ func TestPID_UnmarshalJSON(t *testing.T) {
}
}
}

func TestTask_containerIP(t *testing.T) {
makeTask := func(networkInfoAddress string, label Label) Task {
labels := make([]Label, 0, 1)
if label.Key != "" {
labels = append(labels, label)
}

var containerStatus ContainerStatus
if networkInfoAddress != "" {
containerStatus = ContainerStatus{
NetworkInfos: []NetworkInfo{
NetworkInfo{
IPAddress: networkInfoAddress,
},
},
}
}

return Task{
State: "TASK_RUNNING",
Statuses: []Status{
Status{
Timestamp: 1.0,
State: "TASK_RUNNING",
Labels: labels,
ContainerStatus: containerStatus,
},
},
}
}

// Verify IP extraction from NetworkInfo
task := makeTask("1.2.3.4", Label{})
if task.containerIP("mesos") != "1.2.3.4" {
t.Errorf("Failed to extract IP from NetworkInfo")
}

// Verify IP extraction from NetworkInfo takes precedence over
// labels
task = makeTask("1.2.3.4",
Label{Key: ipLabels["mesos"], Value: "2.4.6.8"})
if task.containerIP("mesos") != "1.2.3.4" {
t.Errorf("Failed to extract IP from NetworkInfo when label also supplied")
}

// Verify IP extraction from the Mesos label without NetworkInfo
task = makeTask("",
Label{Key: ipLabels["mesos"], Value: "1.2.3.4"})
if task.containerIP("mesos") != "1.2.3.4" {
t.Errorf("Failed to extract IP from Mesos label")
}

// Verify IP extraction from the Docker label without NetworkInfo
task = makeTask("",
Label{Key: ipLabels["docker"], Value: "1.2.3.4"})
if task.containerIP("docker") != "1.2.3.4" {
t.Errorf("Failed to extract IP from Docker label")
}
}