Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix result handling of ec2_run_instance #128

Merged
merged 6 commits into from Feb 11, 2016
Merged

Fix result handling of ec2_run_instance #128

merged 6 commits into from Feb 11, 2016

Conversation

manasdk
Copy link
Contributor

@manasdk manasdk commented Feb 10, 2016

Fix to align with changes to AWS pack StackStorm/st2contrib#393

@manasdk manasdk changed the title Fix result handling of ec2_run_instance [DONT MERGE] Fix result handling of ec2_run_instance Feb 10, 2016
@manasdk manasdk force-pushed the fix_per_aws_pack branch 2 times, most recently from 6e2210d to 590946f Compare February 10, 2016 20:02
@Kami
Copy link
Member

Kami commented Feb 11, 2016

Cool.

It would also be a good idea to merge the branch from #126 (destroy_vm_changes) into yours or vice-versa so everything is self contained and easy to test.

@Kami
Copy link
Member

Kami commented Feb 11, 2016

I will merge my PR into this one.

@manasdk
Copy link
Contributor Author

manasdk commented Feb 11, 2016

@Kami Your call on how to merge this branch etc.

@Kami
Copy link
Member

Kami commented Feb 11, 2016

I'm testing those changes and it doesn't look like destroy_vm works correctly - it doesn't seem to delete an instance if one already exists.

In any case, I will let @m4dcoder test and investigate this when he's back since he has more context on the whole Mistral workflow...

@Kami
Copy link
Member

Kami commented Feb 11, 2016

Also, we need to be careful when testing those changes since it would be really bad if the action deleted wrong instance(s).

@Kami
Copy link
Member

Kami commented Feb 11, 2016

OK, so the good news.

I tested those + aws pack changes locally on the build server and it looks like they work - all the workroom tests pass. There might still be some other tests / workflows affected, but we should fix those when they pop up.

I will go ahead, merge both of those changes and deploy them to st2build002.

"destroy_vm" still doesn't look like it works as it should, but I'll leave this to @m4dcoder when he's back.

@Kami Kami changed the title [DONT MERGE] Fix result handling of ec2_run_instance Fix result handling of ec2_run_instance Feb 11, 2016
Kami added a commit that referenced this pull request Feb 11, 2016
[DONT MERGE] Fix result handling of ec2_run_instance
@Kami Kami merged commit 81d10e1 into master Feb 11, 2016
@Kami Kami deleted the fix_per_aws_pack branch February 11, 2016 12:58
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.

None yet

2 participants