Bug #2346

ec2.rb open-uri "open" doesn't like the port argument

Added by Chris Ferry almost 3 years ago. Updated about 1 year ago.

Status:Closed Start date:06/16/2009
Priority:Normal Due date:
Assignee:James Turnbull % Done:

0%

Category:library
Target version:1.5.9
Keywords:ec2 Affected Facter version:
Branch:https://github.com/jamtur01/facter/tree/tickets/master/2346
Votes: 2

Description

ec2.rb(main):032:0* if can_connect?("169.254.169.254","80")
ec2.rb(main):033:1>   metadata
ec2.rb(main):034:1> end
Exception `ArgumentError' at /usr/lib/ruby/1.8/open-uri.rb:32 - illegal access mode 80

So this collection will fail all the time.

Following is what I did to fix:

8,9c8,9
< def can_connect?(ip,port,wait_sec=2)
<  Timeout::timeout(wait_sec) {open(ip, port)}
---
> def can_connect?(ip,wait_sec=2)
>  Timeout::timeout(wait_sec) {open(ip)}
32c32
< if can_connect?("169.254.169.254","80")
---
> if can_connect?("169.254.169.254")
35d34
< 

Related issues

related to Facter - Bug #4526: ec2 facts: error with "cann_connect" Duplicate 08/12/2010

History

Updated by James Turnbull almost 3 years ago

  • Category set to library
  • Status changed from Unreviewed to Accepted
  • Assignee set to James Turnbull
  • Target version set to 1.6.0

Updated by Paul Nasrat almost 3 years ago

What ruby version are you using?

Updated by Paul Nasrat almost 3 years ago

  • Status changed from Accepted to Needs Decision
  • Assignee changed from James Turnbull to Paul Nasrat

This fact is completely broken at the moment:

  • Timeout::Error isn’t caught by rescue (due to how it inherits)
  • The issue of wrong open semantics outlined here, this is causing hidden immediate failure
  • The fact is going to cause a 2 second wait to every facter run

I’ve raised a thread with a patch on list for discussion.

Updated by James Turnbull over 2 years ago

This might be a better version of the fact – http://www.devco.net/archives/2009/11/09/rightscale_facts.php

Updated by R.I. Pienaar over 2 years ago

James Turnbull wrote:

This might be a better version of the fact – http://www.devco.net/archives/2009/11/09/rightscale_facts.php

This one is rightscale specific, the existing one (which I didnt know about) should work on all ec2 machines – though not sure why it didnt for me.

Not sure which is the best approach, but not sure fetching all the meta data every time is the right one eventhough they do seem to change now and then.

Updated by Dis Connect over 2 years ago

FWIW talking with the ubuntu-server guys, some of the meta-data can change during the life of an instance.

Also, this totally hosed up my puppet installation until I removed the rescue filter. (It doesn’t matter -why- we didn’t get metadata. It is sufficient that we didn’t, and we should move on..)

Updated by Nick Lewis almost 2 years ago

There’s a patch for this here:

http://groups.google.com/group/puppet-dev/browse_thread/thread/ab1570b7fc9c49fd/c7fa205e641bec46

But it mentions causing a delay, which seems not to have been addressed.

Updated by James Turnbull almost 2 years ago

Is there any way to confine the fact to EC2?

Updated by Paul Nasrat almost 2 years ago

There are a few approaches you could do to limit the systems exposed to?

This suggests:

  • Am I on xen
  • am I on 10/8 (although not sure that works with Virtual Private Cloud)
  • does an arp entry for 169.254.169.254 exist

http://developer.amazonwebservices.com/connect/message.jspa?messageID=122591

So we probably want an in_ec2 based on that and then get the meta-data facts.

Updated by James Turnbull almost 2 years ago

Paul – how about this code:

http://bazaar.launchpad.net/~eythian/+junk/ec2facts/annotate/head:/facts/ec2facts.rb

Updated by R.I. Pienaar almost 2 years ago

James Turnbull wrote:

Paul – how about this code:

http://bazaar.launchpad.net/~eythian/+junk/ec2facts/annotate/head:/facts/ec2facts.rb

I think these 2 solve different problems. The original facter fact will take all the meta data and add facts for them. The code in this link will add facts from the user data and only the instanceid nothing else.

This script above works but: i had to change the URLs to not be a date specific instead ‘latest’ also the path to arp isnt always right and the IP for retrieving the information will only be in the ARP cache if there’s been a connection attempt so a fresh boot wont have it.

Updated by Paul Nasrat almost 2 years ago

RI – note that I believe most AMI’s get their root ssh keys from 169.254.169.254 eg /usr/local/sbin/get-credentials.sh in the Amazon fedora images. Obviously this may not be true from custom built AMIs, but even then they probably do grab user-data in early boot.

Problem is we need to care about linux, opensolaris, windows.

Updated by James Turnbull over 1 year ago

  • Branch set to http://github.com/LiorCohen/facter/commit/3113d2ad94d428319817c2aea3ef20accbd3c349

Updated by Patrick Ramsey over 1 year ago

Nick’s patch is the correct one. It doesn’t introduce a 2 second timeout; it enables one which was already accepted and is supposed to be happening.

Chris’s original patch does not actually fix the issue; the issue is that open-uri only monkeypatches open() to be able to recognize non-file:/// uri schemas, not to take TCPSocket.open() semantics. Eliminating the second argument (which open() sees as the mode) does not solve the problem, as it still sees “169.254.169.254” as a file name. It should be called with “http://#{ip}:#{port}/” as its only argument.

Nick Lewis wrote:

There’s a patch for this here:

http://groups.google.com/group/puppet-dev/browse_thread/thread/ab1570b7fc9c49fd/c7fa205e641bec46

But it mentions causing a delay, which seems not to have been addressed.

Updated by Paul Nasrat about 1 year ago

Technically it’s my patch.

I’ll sync/rediff, and resend for review.

Updated by Paul Nasrat about 1 year ago

  • Status changed from Needs Decision to Accepted

Updated by James Turnbull about 1 year ago

  • Target version changed from 1.6.0 to 1.5.9

Updated by James Turnbull about 1 year ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Assignee changed from Paul Nasrat to James Turnbull
  • Branch changed from http://github.com/LiorCohen/facter/commit/3113d2ad94d428319817c2aea3ef20accbd3c349 to https://github.com/jamtur01/facter/tree/tickets/master/2346

Updated by James Turnbull about 1 year ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

commit:d62e079489c07201cb343f2ca109fecd62d6e567

Updated by Jacob Helwig about 1 year ago

  • Status changed from Merged - Pending Release to Closed

Released in 1.5.9.

Also available in: Atom PDF