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 2016/11/14 17:43:44 UTC

[GitHub] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

GitHub user aledsage opened a pull request:

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

    BROOKLYN-325: alert if provisioning/termination aborted on rebind

    

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

    $ git pull https://github.com/aledsage/brooklyn-server BROOKLYN-325-handle-lost-vm

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

    https://github.com/apache/brooklyn-server/pull/435.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 #435
    
----
commit 182a149361dc259a6b2bf00c51c62d742df0a4fc
Author: Aled Sage <al...@gmail.com>
Date:   2016-11-14T17:41:06Z

    Move INTERNAL_PROVISIONING_TASK_STATE to AttributesInternal
    
    So that it can subsequently be used to fix BROOKLYN-325.

commit 47a230445219f93f56c763d5f385157b2bdbac7b
Author: Aled Sage <al...@gmail.com>
Date:   2016-11-14T17:41:13Z

    squash

commit 85a8c2a5f042f2ed001263dad315faef99222ba0
Author: Aled Sage <al...@gmail.com>
Date:   2016-11-14T17:42:36Z

    BROOKLYN-325: alert if provisioning/termination aborted on rebind

----


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88179580
  
    --- Diff: core/src/main/resources/org/apache/brooklyn/core/mgmt/persist/deserializingClassRenames.properties ---
    @@ -1434,3 +1434,5 @@ brooklyn.test.entity.BlockingEntityImpl
     org.apache.brooklyn.config.ConfigInheritance$None                                : org.apache.brooklyn.config.ConfigInheritance$Legacy$None
     org.apache.brooklyn.config.ConfigInheritance$Always                              : org.apache.brooklyn.config.ConfigInheritance$Legacy$Always
     org.apache.brooklyn.config.ConfigInheritance$Merged                              : org.apache.brooklyn.config.ConfigInheritance$Legacy$Merged
    +
    +org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks$ProvisioningTaskState : org.apache.brooklyn.core.entity.internal.AttributesInternal$ProvisioningTaskState
    --- End diff --
    
    Can you add a comment "since 0.10.0" so we can start removing entries at some point.


---
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] brooklyn-server issue #435: BROOKLYN-325: alert if provisioning/termination ...

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

    https://github.com/apache/brooklyn-server/pull/435
  
    Good to merge. I'll leave it to you to decide whether to clean the indicators in start/restart in this PR. Or leave it to the user to do it from the UI.



---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88292901
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java ---
    @@ -245,14 +249,52 @@ protected void instanceRebind(AbstractBrooklynObject instance) {
             Preconditions.checkState(instance == entity, "Expected %s and %s to match, but different objects", instance, entity);
             Lifecycle expectedState = ServiceStateLogic.getExpectedState(entity);
             if (expectedState == Lifecycle.STARTING || expectedState == Lifecycle.STOPPING) {
    -            LOG.warn("Entity {} goes on-fire because it was in state {} on rebind", entity, expectedState);
    -            LOG.warn("not-up-indicators={}", entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS));
    -            ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +            // If we were previously "starting" or "stopping", then those tasks will have been 
    +            // aborted. We don't want to continue showing that state (e.g. the web-console would
    +            // then show the it as in-progress with the "spinning" icon).
    +            // Therefore we set the entity as on-fire, and add the indicator that says why.
    +            markTransitioningEntityOnFireOnRebind((EntityInternal) entity, expectedState);
    +        }
    +        
    +        // Clear the provisioning/termination task-state; the task will have been aborted, so wrong to keep this state.
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE);
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_TERMINATION_TASK_STATE);
    +        
    +        super.instanceRebind(instance);
    +    }
    +    
    +    protected void markTransitioningEntityOnFireOnRebind(EntityInternal entity, Lifecycle expectedState) {
    +        LOG.warn("Entity {} being marked as on-fire because it was in state {} on rebind; indicators={}", new Object[] {entity, expectedState, entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS)});
    +        ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(
    --- End diff --
    
    I think `start` & `restart` should clear these particular values, not all. Since the errors are triggered by start, restart, stop actions - these effectors are kind of responsible. So a subsequent start will reset the state and the entity will become healthy.



---
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] brooklyn-server issue #435: BROOKLYN-325: alert if provisioning/termination ...

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

    https://github.com/apache/brooklyn-server/pull/435
  
    Looks good. Tested all the cases and seems to work well.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88233310
  
    --- Diff: core/src/main/resources/org/apache/brooklyn/core/mgmt/persist/deserializingClassRenames.properties ---
    @@ -1434,3 +1434,5 @@ brooklyn.test.entity.BlockingEntityImpl
     org.apache.brooklyn.config.ConfigInheritance$None                                : org.apache.brooklyn.config.ConfigInheritance$Legacy$None
     org.apache.brooklyn.config.ConfigInheritance$Always                              : org.apache.brooklyn.config.ConfigInheritance$Legacy$Always
     org.apache.brooklyn.config.ConfigInheritance$Merged                              : org.apache.brooklyn.config.ConfigInheritance$Legacy$Merged
    +
    +org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks$ProvisioningTaskState : org.apache.brooklyn.core.entity.internal.AttributesInternal$ProvisioningTaskState
    --- End diff --
    
    I think it will be useful to have a marker here as well.  Agree it can be done separately. A good time to begin will be post-0.10.0 release. Then can treat all previous entries as added in 0.10.0.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88242774
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java ---
    @@ -245,14 +249,52 @@ protected void instanceRebind(AbstractBrooklynObject instance) {
             Preconditions.checkState(instance == entity, "Expected %s and %s to match, but different objects", instance, entity);
             Lifecycle expectedState = ServiceStateLogic.getExpectedState(entity);
             if (expectedState == Lifecycle.STARTING || expectedState == Lifecycle.STOPPING) {
    -            LOG.warn("Entity {} goes on-fire because it was in state {} on rebind", entity, expectedState);
    -            LOG.warn("not-up-indicators={}", entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS));
    -            ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +            // If we were previously "starting" or "stopping", then those tasks will have been 
    +            // aborted. We don't want to continue showing that state (e.g. the web-console would
    +            // then show the it as in-progress with the "spinning" icon).
    +            // Therefore we set the entity as on-fire, and add the indicator that says why.
    +            markTransitioningEntityOnFireOnRebind((EntityInternal) entity, expectedState);
    +        }
    +        
    +        // Clear the provisioning/termination task-state; the task will have been aborted, so wrong to keep this state.
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE);
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_TERMINATION_TASK_STATE);
    +        
    +        super.instanceRebind(instance);
    +    }
    +    
    +    protected void markTransitioningEntityOnFireOnRebind(EntityInternal entity, Lifecycle expectedState) {
    +        LOG.warn("Entity {} being marked as on-fire because it was in state {} on rebind; indicators={}", new Object[] {entity, expectedState, entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS)});
    +        ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(
    --- End diff --
    
    Good question. No, start/restart won't clear that not-up-indicator.
    
    In `brooklyn-jsgui` (i.e. the web-console) there is a "reset problems" button in the advanced tab. This makes a couple of API calls for the entity. It sets `service.notUp.indicators` and `service.problems` to be empty maps.
    
    I think that is probably the only way to do this currently!
    
    We need to at least update the docs about this. What would be a better way? Should start() and restart() effectors clear all not-up-indicators? That feels fiddly - it might cause the entity to immediately report `serviceUp=true` (which is why `SoftwareProcessImpl.initEnrichers()` calls `ServiceNotUpLogic.updateNotUpIndicator(this, SERVICE_PROCESS_IS_RUNNING, "No information yet on whether this service is running")`).
    
    Thoughts?


---
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] brooklyn-server issue #435: BROOKLYN-325: alert if provisioning/termination ...

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

    https://github.com/apache/brooklyn-server/pull/435
  
    Thanks, I'll not include clearing the indicators in this PR.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88180158
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java ---
    @@ -245,14 +249,52 @@ protected void instanceRebind(AbstractBrooklynObject instance) {
             Preconditions.checkState(instance == entity, "Expected %s and %s to match, but different objects", instance, entity);
             Lifecycle expectedState = ServiceStateLogic.getExpectedState(entity);
             if (expectedState == Lifecycle.STARTING || expectedState == Lifecycle.STOPPING) {
    -            LOG.warn("Entity {} goes on-fire because it was in state {} on rebind", entity, expectedState);
    -            LOG.warn("not-up-indicators={}", entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS));
    -            ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +            // If we were previously "starting" or "stopping", then those tasks will have been 
    +            // aborted. We don't want to continue showing that state (e.g. the web-console would
    +            // then show the it as in-progress with the "spinning" icon).
    +            // Therefore we set the entity as on-fire, and add the indicator that says why.
    +            markTransitioningEntityOnFireOnRebind((EntityInternal) entity, expectedState);
    +        }
    +        
    +        // Clear the provisioning/termination task-state; the task will have been aborted, so wrong to keep this state.
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE);
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_TERMINATION_TASK_STATE);
    +        
    +        super.instanceRebind(instance);
    +    }
    +    
    +    protected void markTransitioningEntityOnFireOnRebind(EntityInternal entity, Lifecycle expectedState) {
    +        LOG.warn("Entity {} being marked as on-fire because it was in state {} on rebind; indicators={}", new Object[] {entity, expectedState, entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS)});
    +        ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(
    +                entity, 
    +                "Task aborted on rebind", 
    +                "Set to on-fire (from previous expected state "+expectedState+") because tasks aborted on rebind");
    --- End diff --
    
    Tasks get aborted during shutdown.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88179360
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java ---
    @@ -0,0 +1,52 @@
    +/*
    + * or more contributor license agreements.  See the NOTICE file
    --- End diff --
    
    Good spot, interestingly rat doesn't complain.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88055510
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java ---
    @@ -0,0 +1,52 @@
    +/*
    + * or more contributor license agreements.  See the NOTICE file
    --- End diff --
    
    First line is missing from the Licence


---
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] brooklyn-server issue #435: BROOKLYN-325: alert if provisioning/termination ...

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

    https://github.com/apache/brooklyn-server/pull/435
  
    @neykov comments addressed in the new final commit (and rebased against master).


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88180701
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java ---
    @@ -0,0 +1,52 @@
    +/*
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.entity.internal;
    +
    +import org.apache.brooklyn.api.sensor.AttributeSensor;
    +import org.apache.brooklyn.core.entity.Attributes;
    +import org.apache.brooklyn.core.sensor.BasicAttributeSensor;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.reflect.TypeToken;
    +
    +public interface AttributesInternal extends Attributes {
    +    @Beta
    +    public static final AttributeSensor<ProvisioningTaskState> INTERNAL_PROVISIONING_TASK_STATE = new BasicAttributeSensor<ProvisioningTaskState>(
    +            TypeToken.of(ProvisioningTaskState.class), 
    +            "internal.provisioning.task.state",
    +            "Internal transient sensor (do not use) for tracking the provisioning of a machine (to better handle aborting/rebind)");
    +    
    +    @Beta
    +    public static final AttributeSensor<ProvisioningTaskState> INTERNAL_TERMINATION_TASK_STATE = new BasicAttributeSensor<ProvisioningTaskState>(
    +            TypeToken.of(ProvisioningTaskState.class), 
    +            "internal.termination.task.state",
    +            "Internal transient sensor (do not use) for tracking the termination of a machine (to better handle aborting/rebind)");
    +
    +    /**
    +     * Used only internally by {@link org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks}
    +     * to track provisioning/termination. This is used so the machine can be terminated if stopped while opaque provision
    +     * call is being made; and is used to report if termination was prematurely aborted (e.g. during Brooklyn restart).
    +     */
    +    @Beta
    +    @VisibleForTesting
    +    public enum ProvisioningTaskState {
    +        RUNNING,
    +        DONE;
    --- End diff --
    
    What's the point of a `DONE`? Isn't `null` the same?


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88179297
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java ---
    @@ -0,0 +1,52 @@
    +/*
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.entity.internal;
    +
    +import org.apache.brooklyn.api.sensor.AttributeSensor;
    +import org.apache.brooklyn.core.entity.Attributes;
    +import org.apache.brooklyn.core.sensor.BasicAttributeSensor;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.reflect.TypeToken;
    +
    +public interface AttributesInternal extends Attributes {
    +    @Beta
    +    public static final AttributeSensor<ProvisioningTaskState> INTERNAL_PROVISIONING_TASK_STATE = new BasicAttributeSensor<ProvisioningTaskState>(
    +            TypeToken.of(ProvisioningTaskState.class), 
    --- End diff --
    
    Could use `Sensors.newSensor(ProvisioningTaskState.class`, below as well.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88292103
  
    --- Diff: core/src/main/resources/org/apache/brooklyn/core/mgmt/persist/deserializingClassRenames.properties ---
    @@ -1434,3 +1434,5 @@ brooklyn.test.entity.BlockingEntityImpl
     org.apache.brooklyn.config.ConfigInheritance$None                                : org.apache.brooklyn.config.ConfigInheritance$Legacy$None
     org.apache.brooklyn.config.ConfigInheritance$Always                              : org.apache.brooklyn.config.ConfigInheritance$Legacy$Always
     org.apache.brooklyn.config.ConfigInheritance$Merged                              : org.apache.brooklyn.config.ConfigInheritance$Legacy$Merged
    +
    +org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks$ProvisioningTaskState : org.apache.brooklyn.core.entity.internal.AttributesInternal$ProvisioningTaskState
    --- End diff --
    
    On a tangential note, we should delete all anonymous classes that already have named replacements and use this mechanism to let rebind know about them.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88180286
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/BasicEntityRebindSupport.java ---
    @@ -245,14 +249,52 @@ protected void instanceRebind(AbstractBrooklynObject instance) {
             Preconditions.checkState(instance == entity, "Expected %s and %s to match, but different objects", instance, entity);
             Lifecycle expectedState = ServiceStateLogic.getExpectedState(entity);
             if (expectedState == Lifecycle.STARTING || expectedState == Lifecycle.STOPPING) {
    -            LOG.warn("Entity {} goes on-fire because it was in state {} on rebind", entity, expectedState);
    -            LOG.warn("not-up-indicators={}", entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS));
    -            ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +            // If we were previously "starting" or "stopping", then those tasks will have been 
    +            // aborted. We don't want to continue showing that state (e.g. the web-console would
    +            // then show the it as in-progress with the "spinning" icon).
    +            // Therefore we set the entity as on-fire, and add the indicator that says why.
    +            markTransitioningEntityOnFireOnRebind((EntityInternal) entity, expectedState);
    +        }
    +        
    +        // Clear the provisioning/termination task-state; the task will have been aborted, so wrong to keep this state.
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_PROVISIONING_TASK_STATE);
    +        ((EntityInternal)entity).sensors().remove(AttributesInternal.INTERNAL_TERMINATION_TASK_STATE);
    +        
    +        super.instanceRebind(instance);
    +    }
    +    
    +    protected void markTransitioningEntityOnFireOnRebind(EntityInternal entity, Lifecycle expectedState) {
    +        LOG.warn("Entity {} being marked as on-fire because it was in state {} on rebind; indicators={}", new Object[] {entity, expectedState, entity.getAttribute(Attributes.SERVICE_NOT_UP_INDICATORS)});
    +        ServiceStateLogic.setExpectedState(entity, Lifecycle.ON_FIRE);
    +        ServiceStateLogic.ServiceNotUpLogic.updateNotUpIndicator(
    --- End diff --
    
    Who gets to clean these entries? Does a start/restart get the entity in a healthy state?


---
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] brooklyn-server issue #435: BROOKLYN-325: alert if provisioning/termination ...

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

    https://github.com/apache/brooklyn-server/pull/435
  
    Good clean up. Some questions above.


---
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] brooklyn-server pull request #435: BROOKLYN-325: alert if provisioning/termi...

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

    https://github.com/apache/brooklyn-server/pull/435#discussion_r88262033
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/entity/internal/AttributesInternal.java ---
    @@ -0,0 +1,52 @@
    +/*
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.entity.internal;
    +
    +import org.apache.brooklyn.api.sensor.AttributeSensor;
    +import org.apache.brooklyn.core.entity.Attributes;
    +import org.apache.brooklyn.core.sensor.BasicAttributeSensor;
    +
    +import com.google.common.annotations.Beta;
    +import com.google.common.annotations.VisibleForTesting;
    +import com.google.common.reflect.TypeToken;
    +
    +public interface AttributesInternal extends Attributes {
    +    @Beta
    +    public static final AttributeSensor<ProvisioningTaskState> INTERNAL_PROVISIONING_TASK_STATE = new BasicAttributeSensor<ProvisioningTaskState>(
    +            TypeToken.of(ProvisioningTaskState.class), 
    +            "internal.provisioning.task.state",
    +            "Internal transient sensor (do not use) for tracking the provisioning of a machine (to better handle aborting/rebind)");
    +    
    +    @Beta
    +    public static final AttributeSensor<ProvisioningTaskState> INTERNAL_TERMINATION_TASK_STATE = new BasicAttributeSensor<ProvisioningTaskState>(
    +            TypeToken.of(ProvisioningTaskState.class), 
    +            "internal.termination.task.state",
    +            "Internal transient sensor (do not use) for tracking the termination of a machine (to better handle aborting/rebind)");
    +
    +    /**
    +     * Used only internally by {@link org.apache.brooklyn.entity.software.base.lifecycle.MachineLifecycleEffectorTasks}
    +     * to track provisioning/termination. This is used so the machine can be terminated if stopped while opaque provision
    +     * call is being made; and is used to report if termination was prematurely aborted (e.g. during Brooklyn restart).
    +     */
    +    @Beta
    +    @VisibleForTesting
    +    public enum ProvisioningTaskState {
    +        RUNNING,
    +        DONE;
    --- End diff --
    
    Good point; I'll delete "done" (it won't be in persisted state because `INTERNAL_PROVISIONING_TASK_STATE` used to have `AttributeSensor.SensorPersistenceMode.NONE`), and change it to use null.


---
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.
---