Bug 486691 - Re-deploying deleted modules with previous errors does not open deployment wizard
Summary: Re-deploying deleted modules with previous errors does not open deployment wi...
Status: RESOLVED FIXED
Alias: None
Product: CFT
Classification: ECD
Component: General (show other bugs)
Version: unspecified   Edit
Hardware: PC Mac OS X
: P3 normal
Target Milestone: 1.0 M3   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: plan
Depends on:
Blocks:
 
Reported: 2016-01-27 20:34 EST by Nieraj Singh CLA
Modified: 2016-02-16 15:50 EST (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Nieraj Singh CLA 2016-01-27 20:34:18 EST
Deploy an app that will generate a host taken error.
Module will show in Servers view with error as expected.
Delete the module from Servers view and deploy the app again (drag/drop or add/remove).
Wizard does not open and tools will deploy the app with the old values.
Deleting the module a second time and redeploying again will then open the wizard and let user enter new values.
Comment 1 Nieraj Singh CLA 2016-02-10 14:13:22 EST
We like to consider fixing this for M3. However, considering there is a workaround of sorts, we can push this to later based on what we decided with the comments below.

The problem here is related to how the deletion operation handles modules that are not deployed.

Some errors, like host taken error, that occur during publish operations result in the associated module not being marked as deployed in the server. Even though the module failed to be correctly published, they will appear in the Servers view with an error marker and "Not Deployed". 

However, if a user attempts to delete this module before trying to re-deploy/re-publish the application via drag/drop or add/remove, the deletion operation will prevent the module from being correctly deleted because there is a check in the delete operation that skips deleting modules if they are not deployed.

In DeleteModulesOperation:


for (IModule module : modules) {
   final CloudFoundryApplicationModule appModule = cloudServer.getExistingCloudModule(module);

   if (appModule == null || !appModule.isDeployed()) {
     continue;
   }

   // Continue with deletion...

}

I believe this check for isDeployed() is related to bug 485228 to prevent existing applications in CF from being deleted that have the same name as an application that is being published but whose publish operation is cancelled by the user.

Unfortunately, this also prevents modules that have errors and are not deployed from being deleted.

To consider a possible solution, in general, what criteria would classify a module as being "deployed"? 

If the application exists in CF, should the module be considered deployed? The module may be out of synch (republish required) or be synched (published/synchronised), but as long as the associated application in CF exists, do we consider the module "deployed"? If we go by this definition, then even if errors occur during the publish operation (like host taken) after the application is created in CF, the module will be considered deployed, and when a user attempts to delete the module with errors, it will not be skipped by the code above. This would solve the problem.
Comment 2 Nieraj Singh CLA 2016-02-10 14:24:23 EST
The issue here is that by skipping the deletion of modules that are not deployed, it may leave the modules in an inconsistent state from a user perspective. The deletion operation deletes the modules from the Servers view, but because of the check in the delete operation above that skips deletion in the server itself, the module still exists in the server "hidden" from the user. 

So when a user attempts to re-deploy the application after deleting it from the Servers view, the publish operation will still find an existing module in the server itself and not open the deployment wizard.
Comment 3 Elson Yuen CLA 2016-02-10 17:20:21 EST
Instead of trying to adjust the definition of what is considered as deploy, I think the main problem that we are seeing is the module list on Servers view is inconsistent with what is actually on the server.  This can be the direction that we can consider to fix.

When we decide to skip the deletion of the module on the host taken error, we are removing the application from the Servers view and the application remains on the server.  I think both steps are correct.  The piece that the current code missed is to force a refresh to restore the external module that represents the application installed on the server.  

If we can fix by adding that missing step, then everything should fall into places.  The next time that we are trying to publish will be causes the same host taken detection logic to take place.
Comment 4 Nieraj Singh CLA 2016-02-10 20:28:06 EST
It could be that the inconsistent list of modules in the server causes wider issues than just this one. Here is another one that was just uncovered that may be related to the list of modules in the server not being correctly synched with CF (e.g. possible existing apps in CF that should appear as external modules in the Servers view but not showing up at all):

https://bugs.eclipse.org/bugs/show_bug.cgi?id=487631
Comment 5 Nieraj Singh CLA 2016-02-16 15:02:56 EST
Fix that updates modules on any error during publish type operations (start, restart, push) seems to solve the issue. Fix has been pushed to master.