Skip to content

Conversation

adrienthebo
Copy link

This backports the changes in the master branch for EC2 facts and updates them to return both structured and flattened ec2 metadata facts.

Jeff McCune and others added 3 commits March 28, 2014 13:40
Without this patch Facter has no clear way to define the EC2 related
facts in a robust and reliable manner.  This is a problem because Facter
needs to run as robustly on nodes that are inside of EC2 as it does on
nodes that are outside of EC2.

The root cause of the problem is that we have no way to introspect the
system to determine if we're on EC2 or not without making a network call
to the metadata server. [1]  This means that either non-EC2 nodes will
suffer a timeout trying to connect to a non-existent metadata server or
EC2 nodes will suffer from facts not being defined.

This patch addresses the problem by adding a new utility method with the
following behavior.  A block is accepted and will be executed only under
the following conditions.  The block should execute whatever code is
necessary to define the EC2 facts themselves.

1: Do not execute if the virtual fact is not "xenu"
2: Do not execute the metadata server does not respond in 100ms after
three consecutive attempts.

[1] https://forums.aws.amazon.com/thread.jspa?threadID=62617
Without this patch the EC2 facts use a number of methods to determine if
they're running on openstack, Amazon EC2, eucalyptus, and others.  These
methods have proved to be lacking robustness.

This is a problem because Facter fails to define facts even when a
Metadata server is available.

This patch addresses the problem by changing the behavior of deciding
when to define the dynamic EC2 facts.  The
Facter::Util::EC2.with_metadata_server method is used with a 50ms
timeout over three consecutive retries.

With this patch applied the EC2 metadata and userdata facts will be
defined whenever the node is has a virtual fact value of "xenu" and
there is a metadata data server available at http://169.254.169.254

This patch also removes the `userdata` and `metadata` methods that were
previous defined as instance methods on the base Object class.

Backport from master to facter-2 by Adrien Thebo <adrien@puppetlabs.com>

Conflicts:
	lib/facter/util/ec2.rb
	spec/unit/ec2_spec.rb
	spec/unit/util/ec2_spec.rb
The previous implementation of the EC2 metadata facts could only produce
flattened version of the EC2 metadata. Since we can now return
structured facts it makes more sense to return the structure data in the
original form.
@puppetcla
Copy link

CLA signed by all contributors.

To maintain compatibility with the existing EC2 facts, we need to return
both a structured version of the EC2 facts as well as flattened
versions. This has the unfortunate side effects of forcing fact
evaluation at fact load time, but between the options of managing a
singleton object, duplicating the API queries, and forcing the
evaluation, the last one seems the least bad.
When testing a resolution, the confines may be tested to confirm
suitability on a given platform and so may be changed without destroying
and recreating the resolution. Because of this we need to recalculate
suitability every time #suitable? method is invoked. In normal operation
fact suitability is only determined once so this should have no
performance impact.
The resolution_type method is needed when reopening a fact definition
with the Facter 2.0 API:

Facter.define_fact(:myfact) do
  define_resolution(:myres) do
    # [...]

The aggregate resolution type implemented this but Util::Resolution did
not, which could cause errors using the new syntax. This corrects the
omission.
@kylog
Copy link

kylog commented Apr 2, 2014

Is it possible to add something to the json schema for the ec facts? I think the answer is no, because we just turn the contents of the uri into a series of facts, so the code doesn't actually know enough to say anything about the format or names of those facts. But wanted to at least ask the question :)

@adrienthebo
Copy link
Author

I think it's possible to update the schema for the structured ec2_metadata and ec2_userdata facts. It will be somewhat difficult to actually verify this since we don't run acceptance tests specifically on EC2, but it could be good for future proofing. That being said the data structure for ec2_metadata is rather amorphous so the schema would have to be pretty vague.

For the flattened ec2_ facts we can probably add schemas for a handful of them that we can expect to exist, but it's a bit of a crapshoot. I would have to look more closely at what metadata is available to make an assessment of that.

Before I launch into a discussion of where this is merged, I think that there are a couple of commits that should be pulled out of this and merged/released in 2.0.2. I'll pull those out into separate PRs shortly.

With respect to where this gets merged, introducing the ec2_metadata fact is a feature and thus should be released in 2.1.0. The first solution is pretty easy - just merge all of this into facter-2 and release it in 2.1. Alternately we could refactor this a bit to remove the ec2_metadata fact and just release flat facts in 2.0.2, and re-add the structured fact in 2.1. Since we've managed to get away from the weirdness that was SemVer in Facter 1.7 I would like to keep a closer adherence to SemVer, at least for a little while.

I also wanted to touch briefly on how this code is structured. This continues to perpetuate the pattern of stuffing all complex logic into Facter::Util and implement everything as class methods. What do you think of having lib/facter/ec2/rest.rb that contains this logic, and perhaps refactor it to create real objects that do the querying rather than implementing everything as a class method?

Lastly, and I just realized this - we still have to commit to the old API for Facter::Util::EC2 since that's in the 2.0.1 release, and we can't just rip that all out since it's backwards incompatible. I think this pretty much forces us to use a new namespace for the new code.

Thoughts?

@kylog
Copy link

kylog commented Apr 2, 2014

Oh awesome, thanks for the great comment. Here are some $.02:

  • punt on schemas for ec2 stuff, at least for now
  • once we have acceptance tests that runs against "gold" facts, we should add an ec2 vm to that mix
  • let's just add the structured facts in 2.1 (i.e. no intermediate step adding flat facts in 2.0.2) - I'm not totally sure I followed your proposal so lmk if I'm off base)
  • yes on using real objects rather than class methods
  • preserving the Facter::Util::EC2 API: sigh that makes me sad but yeah who knows if someone's using that ..., so yes we should preserve it

@adrienthebo
Copy link
Author

Pull request for #resolution_type split into #660

@adrienthebo
Copy link
Author

Closing in favor of GH-661.

@adrienthebo adrienthebo closed this Apr 3, 2014
@adrienthebo adrienthebo deleted the backport/facter-2/fact-185-ec2_metadata branch May 9, 2014 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants