You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2015/02/23 22:45:52 UTC
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
GitHub user aledsage opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/525
Fix: no-hostname, and cluster.replaceMember
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn fix/various-20150220
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/525.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #525
----
commit 5a939a2bb4386a403938d8a50d870fb57239d8df
Author: Aled Sage <al...@gmail.com>
Date: 2015-02-20T14:23:58Z
Fix DynamicCluster.replaceMember for MultiLocation
- When using replaceMember with a SoftwareProcess, the member had
two locations (the MachineProvisioningLocation and the
SshMachineLocation). This was causing an exception to be thrown.
commit ca0e6406a8d202891cefa1baa1bacfd7052f08b6
Author: Aled Sage <al...@gmail.com>
Date: 2015-02-23T17:29:15Z
Add BashCommands.setHostname
- A good implementation for CentOS/RHEL, and a poor implementation
for everything else (because not persisted on reboot).
- Also adds BashCommands.prependToEtcHosts and .appendToEtcHosts
- checkJavaHostnameBug() also checks for no hostname.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#issuecomment-76755672
@rdowner I've incorporated those comments. Can you please review again, and also review the second commit so we can see about merging this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/525
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#issuecomment-76376793
Added a couple more comments. With those I think we're ready to merge the hostname changes (no comment yet on the cluster changes).
Note that my knowledge of setting the hostname is based on my experience in the past, but it's possible that modern distributions do things differently in 2015 and perhaps the recommendations have changed. Nevertheless, the old-style way should still work!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#issuecomment-76914411
@aledsage I'm happy with the hostname changes - this has been an issue for a few entities in the past so it's great to have this fix available in core Brooklyn now. I think it could maybe be reworked in the future - I think maybe my thinking is a bit "old fashioned" and it does seem that OSX does things slightly differently - but what is here will definitely be reliable and fix known problems.
I've also reviewed the cluster changes and they seem sane.
:+1: good to merge. Do you want to squash the hostname commits first?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#issuecomment-76970150
Squashing commits, and then will merge:
pick ca0e640 Add BashCommands.setHostname
fixup f0dfe12 setHostname: incorporate review comments
fixup 0d57a39 setHostname: incorporate more review comments
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#issuecomment-75642639
@richardcloudsoft @grkvlt can you take a look at this please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#discussion_r25499681
--- Diff: core/src/test/java/brooklyn/util/ssh/BashCommandsIntegrationTest.java ---
@@ -361,6 +363,99 @@ public void testWaitForPortFreeWhenFreedAfterStart() throws Exception {
serverSocket.close();
}
}
+
+
+ // Disabled by default because of risk of overriding /etc/hosts in really bad way if doesn't work properly!
+ // As a manual visual inspection test, consider first manually creating /etc/hostsname and /etc/sysconfig/network
+ // so that it looks like debian+ubuntu / CentOS/RHEL.
+ @Test(groups={"Integration"}, enabled=false)
+ public void testSetHostnameUnqualified() throws Exception {
+ runSetHostname("br-"+Identifiers.makeRandomId(8).toLowerCase(), null, false);
+ }
+
+ @Test(groups={"Integration"}, enabled=false)
+ public void testSetHostnameQualified() throws Exception {
+ runSetHostname("br-"+Identifiers.makeRandomId(8).toLowerCase()+".brooklyn.incubator.apache.org", null, false);
+ }
+
+ @Test(groups={"Integration"}, enabled=false)
+ public void testSetHostnameNullDomain() throws Exception {
+ runSetHostname("br-"+Identifiers.makeRandomId(8).toLowerCase(), null, true);
+ }
+
+ @Test(groups={"Integration"}, enabled=false)
+ public void testSetHostnameNonNullDomain() throws Exception {
+ runSetHostname("br-"+Identifiers.makeRandomId(8).toLowerCase(), "brooklyn.incubator.apache.org", true);
+ }
+
+ protected void runSetHostname(String newHostname, String newDomain, boolean includeDomain) throws Exception {
+ String fqdn = (includeDomain && Strings.isNonBlank(newDomain)) ? newHostname + "." + newDomain : newHostname;
+
+ LocalManagementContextForTests mgmt = new LocalManagementContextForTests();
+ SshMachineLocation loc = mgmt.getLocationManager().createLocation(LocalhostMachineProvisioningLocation.spec()).obtain();
+
+ execRequiringZeroAndReturningStdout(loc, sudo("cp /etc/hosts /etc/hosts-orig-testSetHostname")).get();
+ execRequiringZeroAndReturningStdout(loc, BashCommands.ifFileExistsElse0("/etc/hostname", sudo("cp /etc/hostname /etc/hostname-orig-testSetHostname"))).get();
+ execRequiringZeroAndReturningStdout(loc, BashCommands.ifFileExistsElse0("/etc/sysconfig/network", sudo("cp /etc/sysconfig/network /etc/sysconfig/network-orig-testSetHostname"))).get();
+
+ String origHostname = getHostname(loc);
+ assertTrue(Strings.isNonBlank(origHostname));
+
+ try {
+ List<String> cmd = (includeDomain) ? BashCommands.setHostname(newHostname, newDomain) : BashCommands.setHostname(newHostname);
+ execRequiringZeroAndReturningStdout(loc, cmd).get();
+
+ assertEquals(getHostname(loc), fqdn);
+
+ String result = execRequiringZeroAndReturningStdout(loc, "grep -n "+fqdn+" /etc/hosts").get();
+ assertTrue(result.contains("localhost"), "line="+result);
+ log.info("result="+result);
--- End diff --
A good test to do would be:
* Executing `hostname` returns the hostname without the domain name
* Executing `hostname --fqdn` returns the hostname *with* the domain name
* Executing `ping -c1 -n -q ${ hostname }` return success (rc=0)
* Executing `ping -c1 -n -q ${ hostname --fqdn }` return success (rc=0)
The last two are probably the most important - it's these kinds of failures that would also cause an entity to fail to bind.
Unfortunately OSX has slightly different parameters - it's `hostname -s` and `hostname -f` respectively.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#issuecomment-75732888
Hi @aledsage, some comments on the set-hostname commit. I can take a look at the other one later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix: no-hostname, and cluster.rep...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/525#issuecomment-76289703
@rdowner I've incorporated those comments; can you review again please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---