On 09/23/11 - 03:48:23PM, Richard Su wrote:

On 09/23/2011 10:46 AM, Richard Su wrote:

...

This would be the case for rhev and vpshere.

src/app/util/taskomatic.rb | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/app/util/taskomatic.rb b/src/app/util/taskomatic.rb index 84dabd0..155abe6 100644 --- a/src/app/util/taskomatic.rb +++ b/src/app/util/taskomatic.rb @@ -157,7 +157,7 @@ module Taskomatic :hwp_cpu => overrides[:cpu], :hwp_storage => overrides[:storage], :realm_id => (match.realm.external_key rescue nil),

•                       :keyname =>  (instance.instance_key.name))
  

•                       :keyname =>  (instance.instance_key.name rescue nil))
  

  end

  def self.matches(instance)

Self ack, pushed.

So I actually removed this because it was causing problems during testing. In particular, if you introduce a bug in instance_key handling (which I did), the key will just fail to show up completely with no error message. In my opinion, we do want that error message.

I might suggest that instead of this, we either:

1) Use the deltacloud features to see if we can use this, or 2) Have a case statement that only fills in keyname when it is an EC2 type provider.

Obviously 1) is preferable, if we can get it to work.

On 09/26/11 - 05:05:29PM, Jan Provaznik wrote:

On 09/26/2011 04:47 PM, Chris Lalancette wrote:

...

On 09/23/11 - 03:48:23PM, Richard Su wrote:

...

On 09/23/2011 10:46 AM, Richard Su wrote:

...

This would be the case for rhev and vpshere.

src/app/util/taskomatic.rb | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/app/util/taskomatic.rb b/src/app/util/taskomatic.rb index 84dabd0..155abe6 100644 --- a/src/app/util/taskomatic.rb +++ b/src/app/util/taskomatic.rb @@ -157,7 +157,7 @@ module Taskomatic :hwp_cpu => overrides[:cpu], :hwp_storage => overrides[:storage], :realm_id => (match.realm.external_key rescue nil),

•                       :keyname =>   (instance.instance_key.name))
  

•                       :keyname =>   (instance.instance_key.name rescue nil))
  

  end

  def self.matches(instance)

Self ack, pushed.

So I actually removed this because it was causing problems during testing. In particular, if you introduce a bug in instance_key handling (which I did), the key will just fail to show up completely with no error message. In my opinion, we do want that error message.

I might suggest that instead of this, we either:

1. Use the deltacloud features to see if we can use this, or
2. Have a case statement that only fills in keyname when it is an EC2 type

provider.

Obviously 1) is preferable, if we can get it to work.

Hi, I thought this patch just fixes keyname parameter passed to dc client when launching an instance - it's independent on showing/hiding key to user. It just says "if instance has a key defined, send it too" and should be equivalent to code before removing condor:

params passed to condor: keyname = instance.instance_key ? instance.instance_key.name : ''

Yes, that's what I'm talking about. The problem with Richard's solution, however, is that it swallows all exceptions. I'd much rather see something like the above line of code, where at least if there is an (unrelated) exception, we will see it.