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

[JENKINS-33339] Allow root volumes to be deleted on termination #218

Merged
merged 5 commits into from Jun 1, 2017

Conversation

Projects
None yet
4 participants
@medwards
Contributor

medwards commented Sep 6, 2016

@medwards

This comment has been minimized.

Show comment
Hide comment
@medwards

medwards Sep 6, 2016

Contributor

Some things for me to do still:

  • Get it to pass unit tests (still not sure how to get this to work locally)
  • Verify the UI hooks actually work (Its my first time exposing things to the user)
  • Run it in a real environment

If you don't merge beforehand I'll ping y'all when I'm done.

Contributor

medwards commented Sep 6, 2016

Some things for me to do still:

  • Get it to pass unit tests (still not sure how to get this to work locally)
  • Verify the UI hooks actually work (Its my first time exposing things to the user)
  • Run it in a real environment

If you don't merge beforehand I'll ping y'all when I'm done.

@medwards

This comment has been minimized.

Show comment
Hide comment
@medwards

medwards Sep 7, 2016

Contributor

I also change setupEphemeralDeviceMapping and setupCustomDeviceMapping to operate on the device mapping list in the launch specification or run instances request. There's now only one method signature and they append to the list rather than wiping it out.

This also means we can support ephemeral and custom device mapping at the same time now. I'm not sure who has this use-case and it seems like a dramatic change so I didn't include it here.

Contributor

medwards commented Sep 7, 2016

I also change setupEphemeralDeviceMapping and setupCustomDeviceMapping to operate on the device mapping list in the launch specification or run instances request. There's now only one method signature and they append to the list rather than wiping it out.

This also means we can support ephemeral and custom device mapping at the same time now. I'm not sure who has this use-case and it seems like a dramatic change so I didn't include it here.

@medwards

This comment has been minimized.

Show comment
Hide comment
@medwards

medwards Sep 13, 2016

Contributor

We're using the snapshot of this internally now and it looks good.

Contributor

medwards commented Sep 13, 2016

We're using the snapshot of this internally now and it looks good.

@medwards

This comment has been minimized.

Show comment
Hide comment
@medwards

medwards Sep 30, 2016

Contributor

OK, just got back from vacation and am happy to report no leaked volumes and our environment is still stable ^^

Contributor

medwards commented Sep 30, 2016

OK, just got back from vacation and am happy to report no leaked volumes and our environment is still stable ^^

@johnnyshields

This comment has been minimized.

Show comment
Hide comment
@johnnyshields

johnnyshields Sep 30, 2016

Contributor

Looks good to me! @francisu what do you think?

Contributor

johnnyshields commented Sep 30, 2016

Looks good to me! @francisu what do you think?

@medwards

This comment has been minimized.

Show comment
Hide comment
@medwards

medwards Mar 31, 2017

Contributor

I've rebased the branch as we got hit by JENKINS-38481 and didn't have it in our custom build of ec2-plugin.

Contributor

medwards commented Mar 31, 2017

I've rebased the branch as we got hit by JENKINS-38481 and didn't have it in our custom build of ec2-plugin.

@micahlmartin

This comment has been minimized.

Show comment
Hide comment
@micahlmartin

micahlmartin Mar 31, 2017

In addition to this, is there a way to configure the size of the root volume or am I just missing it?

In addition to this, is there a way to configure the size of the root volume or am I just missing it?

@medwards

This comment has been minimized.

Show comment
Hide comment
@medwards

medwards Apr 3, 2017

Contributor

I didn't include anything for it. I assumed it was fixed, at least by instance type.

Contributor

medwards commented Apr 3, 2017

I didn't include anything for it. I assumed it was fixed, at least by instance type.

@francisu francisu merged commit 78ea67a into jenkinsci:master Jun 1, 2017

1 check passed

Jenkins This pull request looks good
Details

Jimilian added a commit to Jimilian/ec2-plugin that referenced this pull request Dec 11, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment