Merge pull request #225 from Selvasundaram/master

Cifs backend integration bug fix 

5 files changed, 105 insertions, 55 deletions

diff --git a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/dialogs/CreateVolumeWizard.java b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/dialogs/CreateVolumeWizard.java
index 2317db9c..a7b7d251 100644
--- a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/dialogs/CreateVolumeWizard.java
+++ b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/dialogs/CreateVolumeWizard.java
@@ -23,6 +23,8 @@ import org.eclipse.jface.wizard.Wizard;
 
 import com.gluster.storage.management.client.VolumesClient;
 import com.gluster.storage.management.console.GlusterDataModelManager;
+import com.gluster.storage.management.console.utils.GUIHelper;
+import com.gluster.storage.management.console.utils.GlusterLogger;
 import com.gluster.storage.management.core.constants.CoreConstants;
 import com.gluster.storage.management.core.model.Volume;
 import com.gluster.storage.management.core.model.Volume.VOLUME_STATUS;
@@ -116,6 +118,14 @@ public class CreateVolumeWizard extends Wizard {
 			MessageDialog.openWarning(getShell(), title,
 					"Volume created, but following error(s) occured: " + errMsg);
 		}
-		GlusterDataModelManager.getInstance().addVolume(newVolume);
+		
+		// Fetching actual volume info (because of partial success)
+		Volume volume = newVolume; 
+		try {
+			volume = volumesClient.getVolume(newVolume.getName());
+		}catch (Exception e) {
+			GlusterLogger.getInstance().error("Fetching volume details failed:" + e.getMessage());
+		}
+		GlusterDataModelManager.getInstance().addVolume(volume);
 	}
 }
diff --git a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java
index f9c07497..c538e431 100644
--- a/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java
+++ b/src/com.gluster.storage.management.console/src/com/gluster/storage/management/console/views/VolumeSummaryView.java
@@ -8,6 +8,7 @@ import java.util.List;
 
 import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.jface.fieldassist.ControlDecoration;
+import org.eclipse.osgi.service.resolver.DisabledInfo;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.custom.BusyIndicator;
 import org.eclipse.swt.custom.CLabel;
@@ -80,7 +81,7 @@ public class VolumeSummaryView extends ViewPart {
 	private String nfsMountInfo;
 	private Label nfsLabel;
 	private String nfs;
-	
+
 	private Label numberOfBricks;
 	private Label totalDiskSpace;
 	private Composite alertsSection;
@@ -108,6 +109,9 @@ public class VolumeSummaryView extends ViewPart {
 				changeNFSStatus(volume.isNfsEnabled());
 				updateBrickChanges(volume);
 				toolbarManager.updateToolbar(volume);
+				cifsCheckbox.setSelection(volume.isCifsEnable());
+				populateCifsUsersText();
+				
 			}
 
 			@Override
@@ -141,7 +145,7 @@ public class VolumeSummaryView extends ViewPart {
 		alertsSection = guiHelper.createSection(form, toolkit, "Alerts", null, 1, false);
 		populateAlertSection();
 	}
-	
+
 	private void populateAlertSection() {
 		List<Alert> alerts = cluster.getAlerts();
 
@@ -175,7 +179,7 @@ public class VolumeSummaryView extends ViewPart {
 				+ " <mount-point>";
 		nfsMountInfo = "mount -t nfs " + firstOnlineServer + ":/" + volume.getName() + " <mount-point>";
 		// TODO: if more than 10 servers...
-		String info = "Server can be any server name in the storage cloud eg. <" + onlineServers + ">"; 
+		String info = "Server can be any server name in the storage cloud eg. <" + onlineServers + ">";
 
 		Composite section = guiHelper.createSection(form, toolkit, "Mounting Information", null, 3, false);
 
@@ -196,8 +200,8 @@ public class VolumeSummaryView extends ViewPart {
 		glusterNfsMountText.setLayoutData(new GridData(GridData.BEGINNING, GridData.VERTICAL_ALIGN_CENTER, false,
 				false, 2, 0));
 
-		changeNFSStatus( nfsCheckBox.getSelection());
-		
+		changeNFSStatus(nfsCheckBox.getSelection());
+
 		toolkit.createLabel(section, "");
 		Label infoLabel = toolkit.createLabel(section, info, SWT.NONE);
 		infoLabel.setLayoutData(new GridData(GridData.BEGINNING, GridData.VERTICAL_ALIGN_CENTER, false, false, 2, 0));
@@ -270,17 +274,17 @@ public class VolumeSummaryView extends ViewPart {
 		layoutData.widthHint = 300;
 		return layoutData;
 	}
-	
+
 	private void createCifsField(Composite section) {
 		cifsLabel = toolkit.createLabel(section, "CIFS: ", SWT.NONE);
 		cifsUsersText = toolkit.createText(section, volume.getAccessControlList(), SWT.BORDER);
 		populateCifsUsersText();
 		addKeyListenerForCifsUser();
-		
+
 		cifsUpdateLinkComposite = toolkit.createComposite(section, SWT.NONE);
 		cifsUpdateLinkComposite.setLayout(new FillLayout());
 		cifsUpdateLinkComposite.setVisible(volume.isCifsEnable());
-		
+
 		createChangeLinkForCifs(cifsUpdateLinkComposite);
 		renderCifsUsers(cifsCheckbox.getSelection());
 		errCifsDecoration = guiHelper.createErrorDecoration(cifsUsersText);
@@ -369,7 +373,7 @@ public class VolumeSummaryView extends ViewPart {
 		guiHelper.clearStatusMessage();
 		parent.update();
 	}
-	
+
 	private void createChangeLinkForCifs(Composite section) {
 		cifsChangeLink = toolkit.createHyperlink(section, "change", SWT.NONE);
 		cifsChangeLink.addHyperlinkListener(new HyperlinkAdapter() {
@@ -379,8 +383,10 @@ public class VolumeSummaryView extends ViewPart {
 			}
 
 			private void startEdit() {
-				enableCifsUsersControls(true);
-				cifsUsersText.selectAll();
+				if (cifsCheckbox.getSelection()) {
+					enableCifsUsersControls(true);
+					cifsUsersText.selectAll();
+				}
 			}
 
 			@Override
@@ -395,24 +401,21 @@ public class VolumeSummaryView extends ViewPart {
 			}
 		});
 	}
-	
-	
+
 	private void saveCifsConfiguration() {
 		guiHelper.setStatusMessage("Setting Cifs Configuration...");
 		parent.update();
 
 		final String cifsUsers = cifsUsersText.getText().trim();
-		List<String> userList = volume.getCifsUsers();
-		String configuredUsers = (userList != null) ? StringUtil.collectionToString(userList, ",") : "";
 
 		// To check if no changes in the users list
-		if (! isvalidCifsUser()) {
+		if (!isvalidCifsUser()) {
 			MessageDialog.openError(Display.getDefault().getActiveShell(), "Cifs Configuration",
-					"Please enter cifs user name ");
+					"Please enter cifs users name");
 			enableCifsUsersControls(true);
 			validateCifsUsers();
-		} else if (cifsUsers.equals(configuredUsers)) { // Nothing to do.
-			enableCifsUsersControls(false);
+			// } else if (cifsUsers.equals(configuredUsers)) { // Nothing to do.
+			// enableCifsUsersControls(false);
 		} else {
 			BusyIndicator.showWhile(Display.getDefault(), new Runnable() {
 				@Override
@@ -426,21 +429,20 @@ public class VolumeSummaryView extends ViewPart {
 						MessageDialog.openError(Display.getDefault().getActiveShell(), "Cifs Configuration",
 								e.getMessage());
 						cifsCheckbox.setSelection(volume.isCifsEnable());
+						enableCifsUsersControls(cifsCheckbox.getSelection());
 						populateCifsUsersText();
 					}
 				}
 			});
-		} 
+		}
 		guiHelper.clearStatusMessage();
 		parent.update();
 	}
-	
-	
-	
+
 	private void saveNFSOption() {
 		guiHelper.setStatusMessage("Setting NFS option...");
 		parent.update();
-		
+
 		BusyIndicator.showWhile(Display.getDefault(), new Runnable() {
 			@Override
 			public void run() {
@@ -487,7 +489,7 @@ public class VolumeSummaryView extends ViewPart {
 		}
 		accessControlText.setText(accessControlList);
 	}
-	
+
 	private void addKeyListenerForCifsUser() {
 		cifsUsersText.addKeyListener(new KeyAdapter() {
 			public void keyReleased(KeyEvent key) {
@@ -496,6 +498,9 @@ public class VolumeSummaryView extends ViewPart {
 					// Reset to default
 					populateCifsUsersText();
 					enableCifsUsersControls(false);
+					if (cifsUsersText.getText().trim().length() == 0) { // Disable CIFS as well
+						cifsCheckbox.setSelection(false);
+					}
 					form.reflow(true);
 					break;
 				case 13:
@@ -503,11 +508,11 @@ public class VolumeSummaryView extends ViewPart {
 					saveCifsConfiguration();
 					break;
 				}
-				// validateCifsUsers();
+				validateCifsUsers();
 			}
 		});
 	}
-	
+
 	private void populateCifsUsersText() {
 		List<String> userList = volume.getCifsUsers();
 		if (volume.isCifsEnable() && userList != null) {
@@ -524,23 +529,33 @@ public class VolumeSummaryView extends ViewPart {
 		nasProtocolsComposite.setLayout(new FillLayout());
 
 		createCheckbox(nasProtocolsComposite, "Gluster", true, false);
-		
+
 		nfsCheckBox = createCheckbox(nasProtocolsComposite, "NFS", volume.isNfsEnabled(), true);
-		
+
 		nfsCheckBox.addSelectionListener(new SelectionAdapter() {
 			@Override
 			public void widgetSelected(SelectionEvent e) {
+				if (!nfsCheckBox.getSelection()) {
+					Integer userAction = new MessageDialog(parent.getShell(), "NFS Re-export", GUIHelper.getInstance()
+							.getImage(IImageKeys.VOLUME_16x16),
+							"Are you sure you want to stop the NFS Re-export for volume [" + volume.getName() + "]?",
+							MessageDialog.QUESTION, new String[] { "No", "Yes" }, -1).open();
+					if (userAction <= 0) { // user select cancel or pressed escape key
+						nfsCheckBox.setSelection(true);
+						return;
+					}
+				}
 				saveNFSOption();
 			}
 		});
-		
+
 		// CIFS checkbox
 		cifsCheckbox = createCheckbox(nasProtocolsComposite, "CIFS", volume.isCifsEnable(), true);
 		createCifsCheckboxListner(cifsCheckbox);
-	
+
 		toolkit.createLabel(section, "", SWT.NONE); // dummy
 	}
-	
+
 	private void createCifsCheckboxListner(final Button cifsCheckbox) {
 		cifsCheckbox.addSelectionListener(new SelectionAdapter() {
 			@Override
@@ -549,9 +564,9 @@ public class VolumeSummaryView extends ViewPart {
 					// need to enable cifs
 					// TODO: Open the text box (empty and enabled),
 					// the hyperlink next to the textbox should have label "update"
-					// when user clicks on that hyperlink, 
+					// when user clicks on that hyperlink,
 					// saveCifsConfiguration should be called
-					// Also, if user presses the "ESC" key, 
+					// Also, if user presses the "ESC" key,
 					// return to the previous state of checkbox and hide the textbox + hyperlink
 					showCifsUsersControls(true);
 					enableCifsUsersControls(true);
@@ -559,15 +574,24 @@ public class VolumeSummaryView extends ViewPart {
 				} else {
 					// need to disable cifs
 					// TODO: hide the textbox and the link AFTER disabling cifs
-					showCifsUsersControls(false);
-					enableCifsUsersControls(false);
+					Integer userAction = new MessageDialog(parent.getShell(), "CIFS Re-export", GUIHelper.getInstance()
+							.getImage(IImageKeys.VOLUME_16x16),
+							"Are you sure you want to stop the CIFS re-export for volume [" + volume.getName() + "]?",
+							MessageDialog.QUESTION, new String[] { "No", "Yes" }, -1).open();
+					if (userAction <= 0) { // user select cancel or pressed escape key
+						cifsCheckbox.setSelection(true); // back to previous state.
+					} else {
+						showCifsUsersControls(false);
+						enableCifsUsersControls(false);
+						saveCifsConfiguration();
+					}
 				}
 				populateCifsUsersText();
 				form.reflow(true);
 			}
 		});
 	}
-	
+
 	private void renderCifsUsers(Boolean cifsSelection) {
 		if (cifsSelection) {
 			enableCifsUsersControls(false);
@@ -578,18 +602,18 @@ public class VolumeSummaryView extends ViewPart {
 	}
 
 	private void showCifsUsersControls(Boolean visible) {
-		if ( visible) {
+		if (visible) {
 			GridData data = new GridData();
 			data.heightHint = 20;
 			data.widthHint = 100;
 			cifsLabel.setLayoutData(data);
-			
+
 			GridData data1 = new GridData();
 			data1.heightHint = 20;
 			data1.widthHint = 300;
-			
+
 			cifsUsersText.setLayoutData(data1);
-			
+
 			GridData data2 = new GridData();
 			data2.heightHint = 25;
 			data2.widthHint = 75;
@@ -602,13 +626,13 @@ public class VolumeSummaryView extends ViewPart {
 			cifsUsersText.setLayoutData(data);
 			cifsUpdateLinkComposite.setLayoutData(data);
 		}
-		
+
 		cifsLabel.setVisible(visible);
 		cifsUsersText.setVisible(visible);
 		cifsUpdateLinkComposite.setVisible(visible);
 		form.reflow(true);
 	}
-	
+
 	private void enableCifsUsersControls(Boolean enable) {
 		cifsUsersText.setEnabled(enable);
 		cifsChangeLink.setText((enable) ? "update" : "change");
@@ -621,20 +645,20 @@ public class VolumeSummaryView extends ViewPart {
 			}
 		}
 	}
-	
+
 	private Button createCheckbox(Composite parent, String label, boolean checked, boolean enabled) {
 		final Button checkBox = toolkit.createButton(parent, label, SWT.CHECK);
 		checkBox.setSelection(checked);
 		checkBox.setEnabled(enabled);
 		return checkBox;
 	}
-	
+
 	private void changeNFSStatus(Boolean isNFSExported) {
 		glusterNfsMountText.setVisible(isNFSExported);
 		nfsLabel.setVisible(isNFSExported);
 		nfsCheckBox.setSelection(isNFSExported);
 	}
-	
+
 	private void updateBrickChanges(Volume volume) {
 		numberOfBricks.setText("" + volume.getNumOfBricks());
 		Double replicaCount = 1d;
@@ -686,7 +710,8 @@ public class VolumeSummaryView extends ViewPart {
 		Double replicaCount = 1d;
 		if (volume.getVolumeType() == VOLUME_TYPE.REPLICATE
 				|| volume.getVolumeType() == VOLUME_TYPE.DISTRIBUTED_REPLICATE) {
-			replicaCount = (double) volume.getReplicaCount();
+			// replicaCount = (double) volume.getReplicaCount();
+			replicaCount = (double) Volume.DEFAULT_REPLICA_COUNT;
 		}
 		totalDiskSpace = toolkit.createLabel(section,
 				"" + NumberUtil.formatNumber((getTotalDiskSpace() / 1024) / replicaCount), SWT.NONE);
@@ -763,19 +788,18 @@ public class VolumeSummaryView extends ViewPart {
 			errDecoration.show();
 		}
 	}
-	
+
 	private void validateCifsUsers() {
+		errCifsDecoration.hide();
 		if (cifsCheckbox.getSelection()) {
 			String cifsUserList = cifsUsersText.getText().trim();
 			if (cifsUserList.length() == 0) {
 				errCifsDecoration.setDescriptionText("Please enter cifs user name");
 				errCifsDecoration.show();
-			} else {
-				errCifsDecoration.hide();
 			}
 		}
 	}
-	
+
 	private boolean isvalidCifsUser() {
 		if (cifsCheckbox.getSelection()) {
 			String cifsUserList = cifsUsersText.getText().trim();
@@ -788,5 +812,5 @@ public class VolumeSummaryView extends ViewPart {
 		validateCifsUsers();
 		return true;
 	}
-	
+
 }
diff --git a/src/com.gluster.storage.management.gateway.scripts/src/gateway/add_user_cifs_all.py b/src/com.gluster.storage.management.gateway.scripts/src/gateway/add_user_cifs_all.py
index 29c182aa..a54685fa 100755
--- a/src/com.gluster.storage.management.gateway.scripts/src/gateway/add_user_cifs_all.py
+++ b/src/com.gluster.storage.management.gateway.scripts/src/gateway/add_user_cifs_all.py
@@ -78,6 +78,7 @@ def main():
     else:
         existingUser = True
 
+    print (serverFile, uid, userName, password)
     rv = Utils.runCommand("grun.py %s add_user_cifs.py %s %s %s" % (serverFile, uid, userName, password))
     if existingUser:
         sys.exit(rv)
diff --git a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/VolumesResource.java b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/VolumesResource.java
index 74275955..e62f7a12 100644
--- a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/VolumesResource.java
+++ b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/resources/v1_0/VolumesResource.java
@@ -210,8 +210,11 @@ public class VolumesResource extends AbstractResource {
 				volumeService.rebalanceStop(clusterName, volumeName);
 			} else if (operation.equals(RESTConstants.FORM_PARAM_CIFS_CONFIG)) {
 				if (enableCifs) {
-					volumeService.modifyCIFSUsers(clusterName, volumeName, cifsUsers);
+					// After add/modify volume cifs users, start/restart the cifs service
+					volumeService.createCIFSUsers(clusterName, volumeName, cifsUsers);
+					volumeService.startCifsReExport(clusterName, volumeName);
 				} else {
+					// Stop the Cifs service and delete the users (!important)
 					volumeService.stopCifsReExport(clusterName, volumeName);
 					volumeService.deleteCifsUsers(clusterName, volumeName);
 				}
diff --git a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java
index bdb81a7f..618c03af 100644
--- a/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java
+++ b/src/com.gluster.storage.management.gateway/src/com/gluster/storage/management/gateway/services/VolumeService.java
@@ -44,6 +44,7 @@ import com.gluster.storage.management.core.model.Brick;
 import com.gluster.storage.management.core.model.GlusterServer;
 import com.gluster.storage.management.core.model.Volume;
 import com.gluster.storage.management.core.model.Volume.NAS_PROTOCOL;
+import com.gluster.storage.management.core.model.Volume.VOLUME_STATUS;
 import com.gluster.storage.management.core.model.Volume.VOLUME_TYPE;
 import com.gluster.storage.management.core.model.VolumeLogMessage;
 import com.gluster.storage.management.core.response.LogMessageListResponse;
@@ -287,6 +288,8 @@ public class VolumeService {
 			ProcessResult result = serverUtil.executeGlusterScript(true, VOLUME_DELETE_CIFS_SCRIPT,
 					file.getAbsolutePath(), volumeName);
 			file.delete();
+			// All the CIFS users deleted, hence stop CIFS service for this volume
+			stopCifsReExport(clusterName, volumeName);
 			if (!result.isSuccess()) {
 				throw new GlusterRuntimeException(result.toString());
 			}
@@ -296,12 +299,20 @@ public class VolumeService {
 		}
 	}
 
-	private void createCIFSUsers(String clusterName, String volumeName, String cifsUsers) {
+	public void createCIFSUsers(String clusterName, String volumeName, String cifsUsers) {
 		try {
 			File file = createOnlineServerList(clusterName);
 			ProcessResult result = serverUtil.executeGlusterScript(true, VOLUME_CREATE_CIFS_SCRIPT,
 					file.getAbsolutePath(), volumeName, cifsUsers.replace(",", " "));
 			file.delete();
+			Volume volume = getVolume(clusterName, volumeName);
+			// If the volume service is already in running, create user may start CIFS re-export automatically.
+			if (volume.getStatus() == VOLUME_STATUS.ONLINE) {
+				startCifsReExport(clusterName, volumeName);
+			} 
+			/*
+			 * else { stopCifsReExport(clusterName, volumeName); }
+			 */
 			if (!result.isSuccess()) {
 				throw new GlusterRuntimeException(result.toString());
 			}
@@ -311,6 +322,7 @@ public class VolumeService {
 		}
 	}
 
+	@Deprecated
 	public void modifyCIFSUsers(String clusterName, String volumeName, String cifsUsers) {
 		try {
 			File file = createOnlineServerList(clusterName);