Add GCE fingerprinting functionality #215

dimfeld · 2015-10-05T05:43:41Z

Also fixes AWS fingerprinter to not run on GCE

This reads the following: * hostname * instance id * machine-type * zone * internal IP * external IP (if any) * tags * attributes Atributes are placed under the platform.gce.attr.* hierarchy. Tags are set up as platform.gce.tag.TagName=true.

GCE and AWS both expose metadata servers, and GCE's 404 response includes the URL in the content, which maatches the regex. So, check the response code as well and if a 4xx code comes back, take that to meanit's not AWS.

dadgar · 2015-10-05T17:54:44Z

client/fingerprint/env_gce.go

+	// provide their own
+	metadataURL := os.Getenv("GCE_ENV_URL")
+	if metadataURL == "" {
+		metadataURL = "http://169.254.169.254/computeMetadata/v1/instance/"


Can you move this string to a const at the top and put a comment on it (make it look less like a magic value)

dadgar · 2015-10-05T18:03:28Z

client/fingerprint/env_gce.go

+		node.Links = make(map[string]string)
+	}
+
+	client := NewGCEMetadataClient(f.logger)


Can we put the client in the EnvGCEFingerprint struct so it doesn't have to be made twice?

Good idea. I originally did it that way so that the client wouldn't stay around forever once it was no longer needed, but I see that the Fingerprint objects aren't persistent anyway.

This allows easier reuse of the same client across multiple functions.

dimfeld · 2015-10-05T19:02:33Z

Above comments should be all taken care of. Just let me know if you have any others :)

…vironments. Fixes hashicorp#224

dadgar · 2015-10-07T17:37:18Z

client/fingerprint/env_gce.go

+	}
+
+	// Get internal and external IP (if it exits)
+	value, err := f.Get("network-interfaces/0/ip", false)


We want to look at all the attached network interfaces. So f.Get("network-interfaces/", true)

* No longer setting Device name in the network interface since we can't match up the info here with real device names. * Add attributes for all external IPs if more than one exists.

dimfeld · 2015-10-07T18:39:56Z

Some comments and outstanding questions on notable changes from the latest push:

Any thoughts on what to do about the network Device names in this case? We potentially end up with multiple interfaces and don't really know what do do about the Device field for each one. The latest code doesn't set Device at all, but I can change that back if you'd like.
Additionally, the fingerprinter is adding a new network interface which overlaps with the interface added by network_unix.go (the AWS fingerprinter has the same problem, I think). A quick browse of the source code makes it look like this isn't actually an issue but it feels a bit odd to end up with two structures that both cover the same network device.
The new format for external IPs is platform.gce.network.NETWORKNAME.external-ip.INDEX where index is 0, 1, 2, etc. This allows us to support multiple values.

FWIW I haven't been able to figure out how to actually attach more than 1 network to a GCE node so most of the above may be theoretical for now.

cbednarski · 2015-10-09T07:10:44Z

Any thoughts on what to do about the network Device names in this case? We potentially end up with multiple interfaces and don't really know what do do about the Device field for each one. The latest code doesn't set Device at all, but I can change that back if you'd like.

I don't know what this means in GCE, but let me give you an example use-case to put this in context.

In AWS some instances have enhanced networking, which makes the network device on the host directly available to the VM (it is not virtualized at all). This provides lower network latency. In Nomad I might want to schedule a latency-sensitive workload on these types of nodes. If the fingerprinter identifies the device type, I can use that information to make placement decisions.

Additionally, the fingerprinter is adding a new network interface which overlaps with the interface added by network_unix.go (the AWS fingerprinter has the same problem, I think). A quick browse of the source code makes it look like this isn't actually an issue but it feels a bit odd to end up with two structures that both cover the same network device.

In this scenario the fingerprinter that has more information (AWS or GCE in this case) should replace, update, or supplement data into the existing entry.

This pattern means generic fingerprinters can run everywhere and provide baseline information, while more specialized fingerprinter can come in afterward and provide more detailed information.

The new format for external IPs is platform.gce.network.NETWORKNAME.external-ip.INDEX where index is 0, 1, 2, etc. This allows us to support multiple values.

Can you clarify the meaning of "network"? Is that a network subnet, or cidr range, or a security group, or just a GCE identifier? I would lean towards mirroring whatever the representation of this data is in GCE's API so it's more intuitive to write placement rules against it.

dimfeld · 2015-10-09T13:40:31Z

In GCE, a "network" is pretty much the equivalent of an AWS VPC. That is, a subnet which is isolated from any other subnets, and with external access through external IPs, address translation, and security-group-like rules.

An instance can only be in a single network; from what I've seen there's no way to create an instance in more than one network/subnet or with more than one interface. I haven't found any options that would expose another network device to the VM such as the AWS option you mentioned.

I think things get confusing here because the GCE metadata API has provisions for instances with multiple network interfaces, and with multiple external IPs per network interface. This was probably done to be forward-compatible, but for now there will never be more than one of each. The main issue with supporting multiple network interfaces is that there isn't really a way to match up the interface for a network named default, for example, to the network device eth0. If we assume there's only one network interface, like it is now, then this is trivial, but otherwise it becomes difficult.

Suggestions

That said, with the current data available there isn't really any extra or better information to add to the Network resources that network_unix.go isn't already finding. So it might be best to just remove all interaction between the GCE fingerprinter and the network resources, and have it only set platform-specific attributes under the platform.gce prefix.

As for the external IP representation, you can see the full data format in the GCEMetadataNetworkInterface struct at the top of the file. The metadata server returns an array of these objects. Part of the issue here is that inside the object is an array of AccessConfigs, but the map[string]string nature of the attributes can't implicitly represent an array, and once again there can only be one right now but that might not be true forever.

Mirroring it exactly would lead to something like platform.gce.network.NETWORKNAME.access-configs.N.external-ip, which we could do but is rather verbose. I'm personally leaning more toward platform.gce.network.external-ip.N right now, but I don't feel strongly about it and will accede to whatever format you'd like.

* cpu-platform * scheduling.automatic-restart * scheduling.on-host-maintenance * network.NETWORKNAME=true

It has nothing to add that the generic fingerprinters aren't finding on their own already.

dimfeld · 2015-10-09T23:41:55Z

I see in feasible.go that there are already plans to implement a "contains" operator. Given that, one way to represent arrays would be as a single attribute containing a comma-separated string. Personally I'm fine with that, but will await your thoughts before messing with it more.

dadgar · 2015-10-12T21:48:00Z

client/fingerprint/env_gce.go

+	metadataURL string
+}
+
+// NewEnvGCEFingerprint is used to create a CPU fingerprint


Update comment.

dimfeld · 2015-10-12T22:03:58Z

@dadgar seems like your last comment disappeared (deleted?) but here's the response I wrote:

The data at that link is what's accessible through the full GCE API though, not through the metadata server, and we don't necessarily have access to the full API (I'm not sure if it's even possible without API keys). Unfortunately much less is exposed through the metadata server endpoints: https://cloud.google.com/compute/docs/metadata?hl=en

As for the IPs, network_unix.go should be picking up the internal IP already anyway on GCE, since that's what Linux is aware of and what shows up in the ifconfig output and so on. There isn't any visibility from the VM as to what its external IP is on GCE except through the metadata server since whatever NAT work is done to make that work takes place outside the VM; AWS works the same way I believe.

dadgar · 2015-10-12T22:54:45Z

client/fingerprint/env_gce.go

+	if err := json.Unmarshal([]byte(value), &interfaces); err != nil {
+		f.logger.Printf("[WARN] fingerprint.env_gce: Error decoding network interface information: %s", err.Error())
+	} else {
+		for _, intf := range interfaces {


@dimfeld: Yeah I deleted because I realized exactly what you wrote. I spent some time reading the GCE docs and create an instance to play with and I agree with what you did in this PR. We will just use the network_unix.go. Just small comments and then we will merge this!

Cool sounds great! Just took care of the one above.

dadgar · 2015-10-12T23:14:27Z

Awesome! Thanks @dimfeld

Add GCE fingerprinting functionality

Ensure installSnapshot consume stream. fixes issue hashicorp#212

github-actions · 2023-05-07T02:14:30Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

dimfeld added 2 commits October 5, 2015 00:40

Fingerprinting code for GCE nodes

9faa553

This reads the following: * hostname * instance id * machine-type * zone * internal IP * external IP (if any) * tags * attributes Atributes are placed under the platform.gce.attr.* hierarchy. Tags are set up as platform.gce.tag.TagName=true.

isAWS should return false on GCE

444d6f6

GCE and AWS both expose metadata servers, and GCE's 404 response includes the URL in the content, which maatches the regex. So, check the response code as well and if a 4xx code comes back, take that to meanit's not AWS.

dadgar reviewed Oct 5, 2015
View reviewed changes

dimfeld added 2 commits October 5, 2015 12:57

Use a constant for the default GCE metadata URL

458adfa

And add a comment to the constant.

69ec26e

dadgar reviewed Oct 5, 2015
View reviewed changes

Consolidate GCEMetadataClient into EnvGCEFingerprint

983e851

This allows easier reuse of the same client across multiple functions.

achanda mentioned this pull request Oct 7, 2015

Local tests are failing in env_aws_test.go #224

Closed

dimfeld added 2 commits October 6, 2015 21:16

NonXXX tests should pass when actually running in their respective en…

a55bdd3

…vironments. Fixes hashicorp#224

TestNetworkFingerprint_notAWS passes even when actually on AWS

f985ef4

dadgar reviewed Oct 7, 2015
View reviewed changes

dimfeld added 2 commits October 7, 2015 12:39

Compact tags and attribute reading code.

79531fe

Parse information for all GCE network interface.

a95cedb

* No longer setting Device name in the network interface since we can't match up the info here with real device names. * Add attributes for all external IPs if more than one exists.

dimfeld added 2 commits October 9, 2015 09:10

Add a few more GCE-specific attributes:

6acdd9d

* cpu-platform * scheduling.automatic-restart * scheduling.on-host-maintenance * network.NETWORKNAME=true

GCE fingerprinter no longer updates network resources

203906b

It has nothing to add that the generic fingerprinters aren't finding on their own already.

dadgar reviewed Oct 12, 2015
View reviewed changes

client/fingerprint/env_gce.go

metadataURL string

}

// NewEnvGCEFingerprint is used to create a CPU fingerprint

Copy link

Contributor

dadgar Oct 12, 2015

Choose a reason for hiding this comment

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

Update comment.

Fix old comments and other syntax cleanup

c43978b

dadgar reviewed Oct 12, 2015
View reviewed changes

More syntax cleanup

263c5f5

dadgar added a commit that referenced this pull request Oct 12, 2015

Merge pull request #215 from Carevoyance/gce_fingerprint

10fe21c

Add GCE fingerprinting functionality

dadgar merged commit 10fe21c into hashicorp:master Oct 12, 2015

dimfeld deleted the gce_fingerprint branch October 13, 2015 01:55

benbuzbee pushed a commit to benbuzbee/nomad that referenced this pull request Jul 21, 2022

Merge pull request hashicorp#215 from superfell/library-v2-stage-one

e5e581e

Ensure installSnapshot consume stream. fixes issue hashicorp#212

github-actions bot locked as resolved and limited conversation to collaborators May 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GCE fingerprinting functionality #215

Add GCE fingerprinting functionality #215

dimfeld commented Oct 5, 2015

dadgar Oct 5, 2015

dadgar Oct 5, 2015

dimfeld Oct 5, 2015

dimfeld commented Oct 5, 2015

dadgar Oct 7, 2015

dimfeld commented Oct 7, 2015

cbednarski commented Oct 9, 2015

dimfeld commented Oct 9, 2015

dimfeld commented Oct 9, 2015

dadgar Oct 12, 2015

dimfeld commented Oct 12, 2015

dadgar Oct 12, 2015

dadgar Oct 12, 2015

dimfeld Oct 12, 2015

dadgar commented Oct 12, 2015

github-actions bot commented May 7, 2023

Add GCE fingerprinting functionality #215

Add GCE fingerprinting functionality #215

Conversation

dimfeld commented Oct 5, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimfeld commented Oct 5, 2015

Choose a reason for hiding this comment

dimfeld commented Oct 7, 2015

cbednarski commented Oct 9, 2015

dimfeld commented Oct 9, 2015

Suggestions

dimfeld commented Oct 9, 2015

Choose a reason for hiding this comment

dimfeld commented Oct 12, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dadgar commented Oct 12, 2015

github-actions bot commented May 7, 2023