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

Adding the T2 class of burstable instances #610

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

lightcapda
Copy link

No description provided.

@cloudbees-pull-request-builder

asgard-pull-requests #464 FAILURE
Looks like there's a problem with this pull request

@cloudbees-pull-request-builder

asgard-pull-requests #465 FAILURE
Looks like there's a problem with this pull request

@marco-hoyer
Copy link

Looks like line length coding-style test is the only thing here. Would really like to get this merged. Is there anything other to do?

@danveloper
Copy link
Contributor

Hi @lightcapda and @marco-hoyer ... First of all, thanks for taking the time to put this together. We're in the process of a major architectural revamping for our Continuous Delivery/Deployment tooling... Some background here: #486 (comment) ... In the meanwhile, we don't have any resources dedicated to enhancing Asgard, only providing bug fixes.

That said, if you guys wanted to take the time to fix the tests and you can find somebody from the community to give some thorough testing to this feature, we can roll it up into this repo. Maybe @pas256 or @e0d would have some free bandwidth to test this branch when the build passes?

@lightcapda
Copy link
Author

We needed to make this change to utilize t2's for our own deployments. I updated our lower environment Asgard instance yesterday and will be conducting a few tests myself. If anything odd comes up I'll delve into it.

@e0d
Copy link
Contributor

e0d commented Aug 6, 2014

@danveloper I can look at this a little over the next couple days, but say that it looks good, what would be the process for merging this and cutting a new release? Also, since this is a breaking change for users of legacy micros, there would need to be change log updates, etc.

@cloudbees-pull-request-builder

asgard-pull-requests #466 SUCCESS
This pull request looks good

@danveloper
Copy link
Contributor

@e0d you rock! Just curious, what is the breaking change here? Looks like t1 support is retained, no?

@e0d
Copy link
Contributor

e0d commented Aug 6, 2014

Ah my fault, I though the had been removed, no breaking change.

@danveloper
Copy link
Contributor

@e0d when you say this is gtg, I'll merge it

@e0d
Copy link
Contributor

e0d commented Aug 11, 2014

@lightcapda An issue I see here is that attempts to launch clusters using t2.micros that result in errors, either because the instance isn't in a vpc or the wrong virtualization type is in use, silently fail.

@lightcapda
Copy link
Author

@e0d I'll take a look when I get a chance. I've mostly been launching t2's within a VPC.

@marco-hoyer
Copy link

I recently tested the changes in this pull-request and couldn't find any issues. Started multiple t2.* instances inside vpc without any problem. If I try to do it without vpc, there is a nice error message in the ASG events view reporting that the instance needs a vpc. Works like I would expect it to do. @e0d, can you give a deeper explanation of your problem?

@cloudbees-pull-request-builder

asgard-pull-requests #482 ABORTED

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.

5 participants