Whirr
  1. Whirr
  2. WHIRR-398

Implement the execution of scripts on DestroyClusterAction

    Details

    • Type: New Feature New Feature
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.7.0
    • Component/s: core
    • Labels:
      None

      Description

      beforeDestroy() method in ClusterActionHandlerSupport does nothing and is misleading.

      Besides doing something on cluster shutdown can be really handy.

      1. WHIRR-398.patch
        35 kB
        David Alves
      2. WHIRR-398.patch
        14 kB
        David Alves
      3. WHIRR-398.patch
        8 kB
        David Alves
      4. WHIRR-398.patch
        8 kB
        David Alves
      5. WHIRR-398.patch
        8 kB
        David Alves
      6. WHIRR-398-unit-test-with-log-dry-run.patch
        2 kB
        David Alves

        Issue Links

          Activity

          Hide
          David Alves added a comment - - edited

          Implements the execution of scripts when DestroyClusterAction is called.

          I have implemented a unit test that makes sure the destroy scripts are executed (after all the configure scripts) but it is uses DryRunModule so it can't go in yet.

          The implementation is nearly a copy/paste of the ConfigureClusterAction but it works.

          Moving forward the common code could/should be extracted possibly even with some parts of BoostrapClusterAction, but maybe that could be done later on another Jira?

          Show
          David Alves added a comment - - edited Implements the execution of scripts when DestroyClusterAction is called. I have implemented a unit test that makes sure the destroy scripts are executed (after all the configure scripts) but it is uses DryRunModule so it can't go in yet. The implementation is nearly a copy/paste of the ConfigureClusterAction but it works. Moving forward the common code could/should be extracted possibly even with some parts of BoostrapClusterAction, but maybe that could be done later on another Jira?
          Hide
          David Alves added a comment -

          Please review/comment.

          Show
          David Alves added a comment - Please review/comment.
          Hide
          David Alves added a comment -

          Removed a rogue syserr and added the patch for the unit test. again the unit test depends on WHIRR-243

          Show
          David Alves added a comment - Removed a rogue syserr and added the patch for the unit test. again the unit test depends on WHIRR-243
          Hide
          David Alves added a comment -

          corrected checkstyle issue. mvn works.

          Show
          David Alves added a comment - corrected checkstyle issue. mvn works.
          Hide
          Andrei Savu added a comment -

          +1 looks good. Can you also open an issue so that we don't forget that we need to extract the common code across actions?

          Show
          Andrei Savu added a comment - +1 looks good. Can you also open an issue so that we don't forget that we need to extract the common code across actions?
          Hide
          Andrei Savu added a comment -

          David I think is good to go. Got some time to look into extracting the common code between actions?

          Show
          Andrei Savu added a comment - David I think is good to go. Got some time to look into extracting the common code between actions?
          Hide
          David Alves added a comment -

          thank you for reviewing.
          I'll have a look into WHIRR-399 today, the configure/destroy part is easy.

          Show
          David Alves added a comment - thank you for reviewing. I'll have a look into WHIRR-399 today, the configure/destroy part is easy.
          Hide
          David Alves added a comment -

          turns out the test on DryRunModuleTest was not asserting correctly and I'm seeing interleaving of configure and destroy messages in dry run.
          will try and finish this asap and leave WHIRR-399 for later (it should merge configure/bootstrap/destroy code)

          Show
          David Alves added a comment - turns out the test on DryRunModuleTest was not asserting correctly and I'm seeing interleaving of configure and destroy messages in dry run. will try and finish this asap and leave WHIRR-399 for later (it should merge configure/bootstrap/destroy code)
          Hide
          Andrei Savu added a comment -

          David can I help with this one? Maybe by writing some tests?

          Show
          Andrei Savu added a comment - David can I help with this one? Maybe by writing some tests?
          Hide
          David Alves added a comment -

          Hi Andrei, the tests are implemented they are just not passing
          I'll have my hands full until thu. but this is next on the pipeline (along with WHIRR-399).

          Show
          David Alves added a comment - Hi Andrei, the tests are implemented they are just not passing I'll have my hands full until thu. but this is next on the pipeline (along with WHIRR-399 ).
          Hide
          David Alves added a comment -

          Ok, tracked down the problem, turns out its not an execution but a DryRunModule problem because NodeMetadata as a strange (broken IMO) equals() contract where the NodeMetadata build for the bootstrap phase is not equals() to the one build for the configure phase because the credentials are different.

          I'm correcting the problem and testing it now.

          Show
          David Alves added a comment - Ok, tracked down the problem, turns out its not an execution but a DryRunModule problem because NodeMetadata as a strange (broken IMO) equals() contract where the NodeMetadata build for the bootstrap phase is not equals() to the one build for the configure phase because the credentials are different. I'm correcting the problem and testing it now.
          Hide
          David Alves added a comment -

          destroyCluster action is now implemented and (unit) tested that is working correctly.

          Its missing integration tests but I'm having trouble justifying the creation of another ITests (new vm's) solely to test destroy cluster action.

          Maybe we could use this in some service?

          Show
          David Alves added a comment - destroyCluster action is now implemented and (unit) tested that is working correctly. Its missing integration tests but I'm having trouble justifying the creation of another ITests (new vm's) solely to test destroy cluster action. Maybe we could use this in some service?
          Hide
          David Alves added a comment -

          minor correction in the test.

          Show
          David Alves added a comment - minor correction in the test.
          Hide
          Andrei Savu added a comment -

          +1 I will add the integration test in WHIRR-409 as discussed.

          Show
          Andrei Savu added a comment - +1 I will add the integration test in WHIRR-409 as discussed.
          Hide
          Andrei Savu added a comment -

          Committed. Thanks David!

          Show
          Andrei Savu added a comment - Committed. Thanks David!

            People

            • Assignee:
              David Alves
              Reporter:
              David Alves
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development