You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by andreaturli <gi...@git.apache.org> on 2017/12/29 21:37:12 UTC

[GitHub] brooklyn-server pull request #926: [WIP] remove PortableTemplateBuilder

GitHub user andreaturli opened a pull request:

    https://github.com/apache/brooklyn-server/pull/926

    [WIP] remove PortableTemplateBuilder

    based on https://github.com/apache/brooklyn-server/pull/807

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/andreaturli/brooklyn-server fix/portableTemplate

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-server/pull/926.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 #926
    
----
commit 07daa7149ed12d6551a8b2212aa44555ed38753d
Author: Andrea Turli <an...@...>
Date:   2017-08-30T12:12:11Z

    remove PortableTemplateBuilder
    
    - adjust MachinePoolPredicates to use Template from jclouds

commit 6579dfb02735b153b98dda996061f3a45cece5f2
Author: andreaturli <an...@...>
Date:   2017-12-29T21:34:08Z

    use jclouds:stub

----


---

[GitHub] brooklyn-server pull request #926: [WIP] remove PortableTemplateBuilder

Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/926#discussion_r160127430
  
    --- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/JcloudsSshMachineLocationStubbedTest.java ---
    @@ -76,29 +76,12 @@ public FailObtainOnPurposeException(String message) {
         @BeforeMethod(alwaysRun=true)
         public void setUp() throws Exception {
             super.setUp();
    -        privateAddresses = ImmutableList.of("172.168.10.11");
    -        publicAddresses = ImmutableList.of("173.194.32.123");
    -        initNodeCreatorAndJcloudsLocation(newNodeCreator(), ImmutableMap.of());
    +        privateAddresses = ImmutableList.of(PRIVATE_IP_ADDRESS);
    +        publicAddresses = ImmutableList.of(PUBLIC_IP_ADDRESS);
    +        jcloudsLocation = initStubbedJcloudsLocation(ImmutableMap.of());
         }
         
    -    @Override
    -    protected NodeCreator newNodeCreator() {
    -        return new AbstractNodeCreator() {
    -            @Override protected NodeMetadata newNode(String group, Template template) {
    -                NodeMetadata result = new NodeMetadataBuilder()
    -                        .id("myid")
    -                        .credentials(LoginCredentials.builder().identity("myuser").credential("mypassword").build())
    -                        .loginPort(22)
    -                        .status(Status.RUNNING)
    -                        .publicAddresses(publicAddresses)
    -                        .privateAddresses(privateAddresses)
    -                        .build();
    -                return result;
    -            }
    -        };
    -    }
    -
    -    @Test
    +    @Test(enabled = false)
    --- End diff --
    
    Is there a reason for disabling this test? If so, please add a comment.


---

[GitHub] brooklyn-server pull request #926: [WIP] remove PortableTemplateBuilder

Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/926#discussion_r160125927
  
    --- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/pool/MachinePoolPredicates.java ---
    @@ -82,40 +83,31 @@ public boolean apply(NodeMetadata input) {
          * (Caveat: If explicit Hardware, Image, and/or Template were specified in the template,
          * then the hash code probably will not detect it.)   
          **/
    -    public static boolean matches(ReusableMachineTemplate template, NodeMetadata m) {
    +    public static boolean matches(Template template, NodeMetadata m) {
             try {
                 // tags and user metadata
     
    -            if (! m.getTags().containsAll( template.getTags(false) )) return false;
    +            if (! m.getTags().containsAll( template.getOptions().getTags())) return false;
     
    -            if (! isSubMapOf(template.getUserMetadata(false), m.getUserMetadata())) return false;
    +            if (! isSubMapOf(template.getOptions().getUserMetadata(), m.getUserMetadata())) return false;
     
     
                 // common hardware parameters
    +            if (m.getHardware().getRam() < template.getHardware().getRam()) return false;
     
    -            if (template.getMinRam()!=null && m.getHardware().getRam() < template.getMinRam()) return false;
    -
    -            if (template.getMinCores()!=null) {
    +            if (template.getHardware().getProcessors().get(0) != null) {
                     double numCores = 0;
                     for (Processor p: m.getHardware().getProcessors()) numCores += p.getCores();
    -                if (numCores+0.001 < template.getMinCores()) return false;
    -            }
    -
    -            if (template.getIs64bit()!=null) {
    -                if (m.getOperatingSystem().is64Bit() != template.getIs64bit()) return false;
    +                if (numCores+0.001 < template.getHardware().getProcessors().get(0).getCores()) return false;
                 }
     
    -            if (template.getOsFamily()!=null) {
    +            if (template.getImage().getOperatingSystem().getFamily() != null) {
                     if (m.getOperatingSystem() == null || 
    -                        !template.getOsFamily().equals(m.getOperatingSystem().getFamily())) return false;
    +                        !template.getImage().getOperatingSystem().getFamily().equals(m.getOperatingSystem().getFamily())) return false;
                 }
    -            if (template.getOsNameMatchesRegex()!=null) {
    --- End diff --
    
    This deleted line does not have an equivalent in the new version. Does this mean a potential a change in behaviour?


---

[GitHub] brooklyn-server issue #926: remove PortableTemplateBuilder

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the issue:

    https://github.com/apache/brooklyn-server/pull/926
  
    retest this please


---

[GitHub] brooklyn-server pull request #926: [WIP] remove PortableTemplateBuilder

Posted by rdowner <gi...@git.apache.org>.
Github user rdowner commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/926#discussion_r160126361
  
    --- Diff: locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java ---
    @@ -2609,7 +2609,7 @@ protected LoginCredentials waitForSshable(
         protected LoginCredentials waitForSshable(
                 HostAndPort hostAndPort, Iterable<LoginCredentials> credentialsToTry, ConfigBag setup) {
             String waitForSshable = setup.get(WAIT_FOR_SSHABLE);
    -        checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForSshable called despite waitForSshable=%s for %s", waitForSshable, hostAndPort);
    +//        checkArgument(!"false".equalsIgnoreCase(waitForSshable), "waitForSshable called despite waitForSshable=%s for %s", waitForSshable, hostAndPort);
    --- End diff --
    
    Please avoid committing commented-out lines of code. Is this line supposed to be deleted, or was it commented out to help debugging?


---