You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2016/02/19 11:07:43 UTC

[6/7] brooklyn-library git commit: Addressing PR comments

Addressing PR comments

Project: http://git-wip-us.apache.org/repos/asf/brooklyn-library/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-library/commit/a01cba0c
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-library/tree/a01cba0c
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-library/diff/a01cba0c

Branch: refs/heads/master
Commit: a01cba0cabb8b3b980c51cd8b90bf9b2b715fe32
Parents: c1a8783
Author: Yavor Yanchev <ya...@yanchev.com>
Authored: Wed Feb 17 11:36:39 2016 +0200
Committer: Yavor Yanchev <ya...@yanchev.com>
Committed: Thu Feb 18 12:54:51 2016 +0200

----------------------------------------------------------------------
 .../entity/cm/ansible/AnsibleBashCommands.java  |  3 +-
 .../entity/cm/ansible/AnsibleConfig.java        |  8 +++-
 .../entity/cm/ansible/AnsibleEntityImpl.java    |  2 -
 .../ansible/AnsibleLifecycleEffectorTasks.java  | 48 +++++++++++++++-----
 .../entity/cm/ansible/AnsiblePlaybookTasks.java |  4 +-
 5 files changed, 46 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/a01cba0c/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleBashCommands.java
----------------------------------------------------------------------
diff --git a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleBashCommands.java b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleBashCommands.java
index e22ee34..ffe74e4 100644
--- a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleBashCommands.java
+++ b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleBashCommands.java
@@ -22,6 +22,7 @@ import static org.apache.brooklyn.util.ssh.BashCommands.INSTALL_CURL;
 import static org.apache.brooklyn.util.ssh.BashCommands.INSTALL_TAR;
 import static org.apache.brooklyn.util.ssh.BashCommands.INSTALL_UNZIP;
 import static org.apache.brooklyn.util.ssh.BashCommands.installExecutable;
+import static org.apache.brooklyn.util.ssh.BashCommands.ifExecutableElse0;
 import static org.apache.brooklyn.util.ssh.BashCommands.sudo;
 
 import org.apache.brooklyn.util.ssh.BashCommands;
@@ -30,7 +31,7 @@ public class AnsibleBashCommands {
 
     public static final String INSTALL_ANSIBLE =
             BashCommands.chain(
-                    sudo("apt-add-repository -y ppa:ansible/ansible"),
+                    ifExecutableElse0("apt-add-repository",sudo("apt-add-repository -y ppa:ansible/ansible")),
                     INSTALL_CURL,
                     INSTALL_TAR,
                     INSTALL_UNZIP,

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/a01cba0c/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleConfig.java
----------------------------------------------------------------------
diff --git a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleConfig.java b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleConfig.java
index d63c2c6..a47ffeb 100644
--- a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleConfig.java
+++ b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleConfig.java
@@ -46,12 +46,16 @@ public interface AnsibleConfig {
     @SetFromFlag("ansible.service.start")
     ConfigKey<String> ANSIBLE_SERVICE_START = ConfigKeys.newStringConfigKey("ansible.service.start",
             "Default start command used with conjunction with the Ansible's service module",
-            "sudo ansible localhost -c local -m service -a \"name=%s state=started\"");
+            "ansible localhost -c local -m service -a \"name=%s state=started\"");
 
     @SetFromFlag("ansible.service.stop")
     ConfigKey<String> ANSIBLE_SERVICE_STOP = ConfigKeys.newStringConfigKey("ansible.service.stop",
             "Default stop command used with conjunction with the Ansible's service module",
-            "sudo ansible localhost -c local -m service -a \"name=%s state=stopped\"");
+            "ansible localhost -c local -m service -a \"name=%s state=stopped\"");
+
+    @SetFromFlag("ansible.service.checkHost")
+    ConfigKey<String> ANSIBLE_SERVICE_CHECK_HOST = ConfigKeys.newStringConfigKey("ansible.service.check.host",
+            "IP to be checked. Default: All IPs ", "0.0.0.0");
 
     @SetFromFlag("ansible.service.checkPort")
     ConfigKey<Integer> ANSIBLE_SERVICE_CHECK_PORT = ConfigKeys.newIntegerConfigKey("ansible.service.check.port");

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/a01cba0c/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleEntityImpl.java
----------------------------------------------------------------------
diff --git a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleEntityImpl.java b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleEntityImpl.java
index c2635ca..43fcd72 100644
--- a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleEntityImpl.java
+++ b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleEntityImpl.java
@@ -41,12 +41,10 @@ public class AnsibleEntityImpl extends EffectorStartableImpl implements AnsibleE
         lifecycleTasks.attachLifecycleEffectors(this);
     }
 
-    @Override
     public void populateServiceNotUpDiagnostics() {
         // TODO no-op currently; should check ssh'able etc
     }
 
-    @Override
     public String ansibleCommand(String module, String args) {
         final ProcessTaskWrapper<Integer> command = DynamicTasks.queue(
             AnsiblePlaybookTasks.moduleCommand(module, config().get(ANSIBLE_VARS), lifecycleTasks.getRunDir(), args));

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/a01cba0c/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleLifecycleEffectorTasks.java
----------------------------------------------------------------------
diff --git a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleLifecycleEffectorTasks.java b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleLifecycleEffectorTasks.java
index e1a8622..e51d9e9 100644
--- a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleLifecycleEffectorTasks.java
+++ b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsibleLifecycleEffectorTasks.java
@@ -35,6 +35,8 @@ import org.apache.brooklyn.location.ssh.SshMachineLocation;
 import org.apache.brooklyn.util.core.task.DynamicTasks;
 import org.apache.brooklyn.util.guava.Maybe;
 import org.apache.brooklyn.util.net.Urls;
+
+import static org.apache.brooklyn.util.ssh.BashCommands.sudo;
 import org.apache.brooklyn.util.text.Strings;
 import org.apache.brooklyn.util.time.Duration;
 import org.apache.brooklyn.util.time.Time;
@@ -99,14 +101,20 @@ public class AnsibleLifecycleEffectorTasks extends MachineLifecycleEffectorTasks
     }
 
     protected void startWithAnsibleAsync() {
+        
         String installDir = Urls.mergePaths(getBaseDir(), "installs/ansible");
 
         String playbookUrl = entity().config().get(ANSIBLE_PLAYBOOK_URL);
         String playbookYaml = entity().config().get(ANSIBLE_PLAYBOOK_YAML);
 
-        if (Strings.isNonBlank(playbookUrl) && Strings.isNonBlank(playbookYaml)) {
-            throw new IllegalArgumentException("You can specify " +  AnsibleConfig.ANSIBLE_PLAYBOOK_URL.getName()
-                +  " or " + AnsibleConfig.ANSIBLE_PLAYBOOK_YAML.getName() + " but not both of them!");
+        if (playbookUrl != null && playbookYaml != null) {
+            throw new IllegalArgumentException( "You can not specify both "+  AnsibleConfig.ANSIBLE_PLAYBOOK_URL.getName() + 
+                " and " + AnsibleConfig.ANSIBLE_PLAYBOOK_YAML.getName() + " as arguments.");
+        }
+
+        if (playbookUrl == null && playbookYaml == null) { 
+                throw new IllegalArgumentException("You have to specify either " + AnsibleConfig.ANSIBLE_PLAYBOOK_URL.getName() + 
+                " or " + AnsibleConfig.ANSIBLE_PLAYBOOK_YAML.getName() + " as arguments.");
         }
 
         DynamicTasks.queue(AnsiblePlaybookTasks.installAnsible(installDir, false));
@@ -138,15 +146,21 @@ public class AnsibleLifecycleEffectorTasks extends MachineLifecycleEffectorTasks
         Maybe<SshMachineLocation> machine = Locations.findUniqueSshMachineLocation(entity().getLocations());
 
         if (machine.isPresent()) {
-            
-            String serviceName = String.format("[%s]%s", entity().config().get(SERVICE_NAME).substring(0, 1),
-                    entity().config().get(SERVICE_NAME).substring(1));
-            String checkCmd = String.format("ps -ef | grep %s", serviceName);
+            // For example “ps -f| grep httpd” matches for any process including the text “httpd”,
+            // which includes the grep command itself, whereas “ps | grep [h]ttpd” matches only processes
+            // including the text “httpd” (doesn’t include the grep) and additionally 
+            // provides a correct return code
+            //
+            // The command constructed bellow will look like  - ps -ef |grep [h]ttpd
+            String serviceNameCheck = getServiceName().replaceFirst("^(.)(.*)", "[$1]$2");
+            String checkCmd = String.format("ps -ef | grep %s", serviceNameCheck);
 
             Integer serviceCheckPort = entity().config().get(ANSIBLE_SERVICE_CHECK_PORT);
 
             if (serviceCheckPort != null) {
-                checkCmd = String.format("sudo ansible localhost -c local -m wait_for -a \"host=0.0.0.0 port=%d\"", serviceCheckPort);
+                checkCmd = sudo(String.format("ansible localhost -c local -m wait_for -a \"host=" + 
+                                        entity().config().get(ANSIBLE_SERVICE_CHECK_HOST) + 
+                                        "\" port=%d\"", serviceCheckPort));
             }
             serviceSshFeed = SshFeed.builder()
                     .entity(entity())
@@ -160,7 +174,8 @@ public class AnsibleLifecycleEffectorTasks extends MachineLifecycleEffectorTasks
                     
              entity().feeds().addFeed(serviceSshFeed);
         } else {
-            LOG.warn("Location(s) {} not an ssh-machine location, so not polling for status; setting serviceUp immediately", entity().getLocations());
+            LOG.warn("Location(s) {} not an ssh-machine location, so not polling for status; "
+                    + "setting serviceUp immediately", entity().getLocations());
         }
     }
 
@@ -169,7 +184,8 @@ public class AnsibleLifecycleEffectorTasks extends MachineLifecycleEffectorTasks
 
         // if it's still up after 5s assume we are good (default behaviour)
         Time.sleep(Duration.FIVE_SECONDS);
-        if (!((Integer)0).equals(DynamicTasks.queue(SshEffectorTasks.ssh(String.format(entity().config().get(AnsibleConfig.ANSIBLE_SERVICE_START), getServiceName()))).get())) {
+        int result = DynamicTasks.queue(SshEffectorTasks.ssh(sudo(getServiveStartCommand()))).get();
+        if (0 != result) {
             throw new IllegalStateException("The process for "+entity()+" appears not to be running (service "+getServiceName()+")");
         }
 
@@ -193,11 +209,19 @@ public class AnsibleLifecycleEffectorTasks extends MachineLifecycleEffectorTasks
 
     protected boolean tryStopService() {
         if (getServiceName()==null) return false;
-        int result = DynamicTasks.queue(SshEffectorTasks.ssh(String.format(entity().config().get(AnsibleConfig.ANSIBLE_SERVICE_STOP), getServiceName()))).get();
+        int result = DynamicTasks.queue(SshEffectorTasks.ssh(sudo(getServiveStopCommand()))).get();
         if (0 == result) return true;
         if (entity().getAttribute(Attributes.SERVICE_STATE_ACTUAL) != Lifecycle.RUNNING)
             return true;
-        
+
         throw new IllegalStateException("The process for "+entity()+" appears could not be stopped (exit code "+result+" to service stop)");
     }
+
+    private String getServiveStartCommand() {
+        return String.format(entity().config().get(AnsibleConfig.ANSIBLE_SERVICE_START), getServiceName());
+    }
+
+    private String getServiveStopCommand() {
+        return String.format(entity().config().get(AnsibleConfig.ANSIBLE_SERVICE_STOP), getServiceName());
+    }
 }

http://git-wip-us.apache.org/repos/asf/brooklyn-library/blob/a01cba0c/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsiblePlaybookTasks.java
----------------------------------------------------------------------
diff --git a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsiblePlaybookTasks.java b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsiblePlaybookTasks.java
index 36076a1..152bc61 100644
--- a/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsiblePlaybookTasks.java
+++ b/software/cm/ansible/src/main/java/org/apache/brooklyn/entity/cm/ansible/AnsiblePlaybookTasks.java
@@ -68,9 +68,9 @@ public class AnsiblePlaybookTasks {
     }
 
     public static TaskFactory<?> runAnsible(final String dir, Object extraVars, String playbookName) {
-        String cmd = String.format("sudo ansible-playbook -i \"localhost,\" -c local "
+        String cmd = sudo(String.format("ansible-playbook -i \"localhost,\" -c local "
             + optionalExtraVarsParameter(extraVars)
-            + " -s %s.yaml", playbookName);
+            + " -s %s.yaml", playbookName));
 
         if (LOG.isDebugEnabled()) {
             LOG.debug("Ansible command: {}", cmd);