Allow service installations to be disabled #12
Conversation
`TODO:` We really ought to stop automatically requiring these files
| serverRole: role | ||
| }; | ||
|
|
||
| $http.post(url, data) |
There was a problem hiding this comment.
why not chain the promise? deferred doesn't seem necessary here
| angular.module('EnvironmentManager.common').factory('targetStateService', function($rootScope, $q, $http) { | ||
|
|
||
| return { | ||
| changeDeploymentStatus: function(enable, service, role, environment) { |
There was a problem hiding this comment.
not necessary to change, but usually with setters / updaters / changers first you would specify target of action, and value (enable) would come last
| @@ -21,8 +21,6 @@ angular.module('EnvironmentManager.environments').controller('DeployModalControl | |||
| }; | |||
|
|
|||
| function init() { | |||
| console.log('Initialising Deploy Controller for ' + parameters.Environment.EnvironmentName); | |||
| @@ -8,7 +8,7 @@ angular.module('EnvironmentManager.environments').component('asgInstances', { | |||
| asgState: '<', | |||
| }, | |||
| controllerAs: 'vm', | |||
| controller: function (awsService, $q, $uibModal) { | |||
| controller: function ($uibModal) { | |||
| return { | ||
| Status: Enums.HEALTH_STATUS.Error, | ||
| Name: checks[0].Name, |
| let servicesList = yield serviceReporter.getServicesList(environmentName, runtimeServerRoleName); | ||
| function getServiceOverallHealth(healthChecks) { | ||
| return { | ||
| Status:_.some(healthChecks, { Status: HEALTH_BAD }) ? HEALTH_BAD : HEALTH_GOOD |
There was a problem hiding this comment.
👍 , just formatting. We'll have also third state, WARNING, in which case logic will be more complex
|
|
||
| let serviceHealthChecks = getServiceChecksInfo(serviceObjects) | ||
| let serviceHealthChecks = getServiceChecksInfo(serviceObjects); | ||
| let Status = service.hasOwnProperty('Status') ? service.Status : 'Enabled'; |
There was a problem hiding this comment.
not sure if that's the best name for enabled / disabled service, status could also point at AWS instance status, might be confusing, especially if we add this to instance
There was a problem hiding this comment.
Suggest something more explicit such as (in the spirit of Powershell DSC) Ensure: 'Absent'|'Present'?
There was a problem hiding this comment.
Definitely open to changing this (particularly if the CDA team has other suggestions / requirements), but remember this is completely separate from AWS and instances, and doesn't ensure absence or presence of a service.
This flag is designed only to stop future installations of a service. Any services that are currently installed will be completely left alone, regardless of a change to the Status value (hence Ensure: 'Absent' would be misleading).
In our Consul key/value store currently, services at this level look like:
{
Name: "ServiceName",
Version: "ServiceVersion",
Slice: "blue|green|none",
DeploymentId: "guid"
}
This new property gets added to the object above. Perhaps InstallationStatus: Enabled | Disabled might be better?
There was a problem hiding this comment.
InstallationStatus sounds good! indicates it's about (further) installation
NewInstallations seems good, too, even more explicit (though soudns funny)
This feature allows the future installation of services to be disabled, without affecting any current infrastructure.
From the Servers > ASG > Services view, individual services can be toggled between enabled or disabled. This sets a
Statusvalue in the key/value store for that particular combination of environment, server role, service, version and slice.To complete this feature, updates need to be made to https://github.com/trainline/consul-deployment-agent that check the
Statusvalue of a service when any new infrastructure is created, and ignore installs for disabled services. Until this is done, the above work is behind a feature flag (FEATURE_DISABLE_SERVICE) that disables the feature by default.To test this feature locally you can run: