Skip to content

Commit

Permalink
fix(cf): Repair Rollback Cluster pipeline stage (#6743)
Browse files Browse the repository at this point in the history
Cloud Foundry's Reactified implementation of AccountRegionClusterSelector
did not account for moniker selection.  The moniker field is required
by Orca to properly perform the rollback.

spinnaker/spinnaker#4180

Co-Authored-By: Stu Pollock <spollock@pivotal.io>
  • Loading branch information
stuart-pollock authored and jkschneider committed Mar 23, 2019
1 parent 9c14b02 commit e0a8639
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 87 deletions.
3 changes: 1 addition & 2 deletions app/scripts/modules/cloudfoundry/src/cf.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { CLOUD_FOUNDRY_ENABLE_ASG_STAGE } from './pipeline/stages/enableAsg/clou
import './pipeline/stages/mapLoadBalancers/cloudfoundryMapLoadBalancersStage.module';
import './pipeline/stages/unmapLoadBalancers/cloudfoundryUnmapLoadBalancersStage.module';
import { CLOUD_FOUNDRY_RESIZE_ASG_STAGE } from './pipeline/stages/resizeAsg/cloudfoundryResizeAsgStage.module';
import { CLOUD_FOUNDRY_ROLLBACK_CLUSTER_STAGE } from './pipeline/stages/rollbackCluster/cloudfoundryRollbackClusterStage.module';
import './pipeline/stages/rollbackCluster/cloudfoundryRollbackClusterStage.module';
import './pipeline/stages/shareService/cloudfoundryShareServiceStage.module';
import './pipeline/stages/unshareService/cloudfoundryUnshareServiceStage.module';
import { CloudFoundryCreateServerGroupModal } from 'cloudfoundry/serverGroup/configure/wizard/CreateServerGroupModal';
Expand All @@ -58,7 +58,6 @@ module(CLOUD_FOUNDRY_MODULE, [
CLOUD_FOUNDRY_LOAD_BALANCER_MODULE,
CLOUD_FOUNDRY_REACT_MODULE,
CLOUD_FOUNDRY_RESIZE_ASG_STAGE,
CLOUD_FOUNDRY_ROLLBACK_CLUSTER_STAGE,
CLOUD_FOUNDRY_SEARCH_FORMATTER,
CLOUD_FOUNDRY_SERVER_GROUP_COMMAND_BUILDER,
CLOUD_FOUNDRY_SERVER_GROUP_TRANSFORMER,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
import * as React from 'react';
import {
AccountService,
Application,
IAccount,
IPipeline,
IRegion,
IStageConfigProps,
StageConfigField,
} from '@spinnaker/core';
import { AccountService, IAccount, IPipeline, IStageConfigProps, StageConfigField } from '@spinnaker/core';

import { AccountRegionClusterSelector } from 'cloudfoundry/presentation';

Expand All @@ -17,13 +9,6 @@ export interface ICloudfoundryRollbackClusterStageProps extends IStageConfigProp

export interface ICloudfoundryRollbackClusterStageConfigState {
accounts: IAccount[];
application: Application;
cloudProvider: string;
credentials: string;
pipeline: IPipeline;
regions: IRegion[];
targetHealthyRollbackPercentage: number;
waitTimeBetweenRegions: number;
}

export class CloudfoundryRollbackClusterStageConfig extends React.Component<
Expand All @@ -33,49 +18,39 @@ export class CloudfoundryRollbackClusterStageConfig extends React.Component<
constructor(props: ICloudfoundryRollbackClusterStageProps) {
super(props);

Object.assign(props.stage, {
this.props.updateStageField({
cloudProvider: 'cloudfoundry',
regions: this.props.stage.regions || [],
targetHealthyRollbackPercentage: 100,
});

this.props.stage.regions = this.props.stage.regions || [];

this.state = {
accounts: [],
application: props.application,
cloudProvider: 'cloudfoundry',
credentials: props.stage.credentials,
pipeline: props.pipeline,
regions: [],
targetHealthyRollbackPercentage: props.stage.targetHealthyRollbackPercentage,
waitTimeBetweenRegions: props.stage.waitTimeBetweenRegions,
};
this.state = { accounts: [] };
}

public componentDidMount = (): void => {
AccountService.listAccounts('cloudfoundry').then(accounts => {
this.setState({ accounts: accounts });
this.setState({ accounts });
});
this.props.stageFieldUpdated();
};

private waitTimeBetweenRegionsUpdated = (event: React.ChangeEvent<HTMLInputElement>): void => {
const time = parseInt(event.target.value || '0', 10);
this.setState({ waitTimeBetweenRegions: time });
this.props.stage.waitTimeBetweenRegions = time;
this.props.stageFieldUpdated();
this.props.updateStageField({ waitTimeBetweenRegions: time });
};

private componentUpdate = (stage: any): void => {
this.props.stage.credentials = stage.credentials;
this.props.stage.regions = stage.regions;
this.props.stage.cluster = stage.cluster;
this.props.stageFieldUpdated();
this.props.updateStageField({
credentials: stage.credentials,
regions: stage.regions,
cluster: stage.cluster,
moniker: stage.moniker,
});
};

public render() {
const { stage } = this.props;
const { application, pipeline, stage } = this.props;
const { waitTimeBetweenRegions } = stage;
const { accounts, application, pipeline } = this.state;
const { accounts } = this.state;
return (
<div className="form-horizontal">
{!pipeline.strategy && (
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,36 +1,17 @@
import { IController, IScope, module } from 'angular';
import { react2angular } from 'react2angular';

import { CloudfoundryRollbackClusterStageConfig } from './CloudfoundryRollbackClusterStageConfig';
import { Application, IStage, Registry } from '@spinnaker/core';

class CloudFoundryRollbackClusterStageCtrl implements IController {
public static $inject = ['$scope', 'application'];
constructor(public $scope: IScope, private application: Application) {
this.$scope.application = this.application;
}
}
import { IStage, Registry } from '@spinnaker/core';

export const CLOUD_FOUNDRY_ROLLBACK_CLUSTER_STAGE = 'spinnaker.cloudfoundry.pipeline.stage.rollbackClusterStage';
module(CLOUD_FOUNDRY_ROLLBACK_CLUSTER_STAGE, [])
.config(function() {
Registry.pipeline.registerStage({
accountExtractor: (stage: IStage) => stage.context.credentials,
configAccountExtractor: (stage: IStage) => [stage.credentials],
provides: 'rollbackCluster',
key: 'rollbackCluster',
cloudProvider: 'cloudfoundry',
templateUrl: require('./cloudfoundryRollbackClusterStage.html'),
controller: 'cfRollbackClusterStageCtrl',
validators: [
{ type: 'requiredField', fieldName: 'cluster' },
{ type: 'requiredField', fieldName: 'regions' },
{ type: 'requiredField', fieldName: 'credentials', fieldLabel: 'account' },
],
});
})
.component(
'cfRollbackClusterStage',
react2angular(CloudfoundryRollbackClusterStageConfig, ['application', 'pipeline', 'stage', 'stageFieldUpdated']),
)
.controller('cfRollbackClusterStageCtrl', CloudFoundryRollbackClusterStageCtrl);
Registry.pipeline.registerStage({
accountExtractor: (stage: IStage) => stage.context.credentials,
configAccountExtractor: (stage: IStage) => [stage.credentials],
provides: 'rollbackCluster',
key: 'rollbackCluster',
cloudProvider: 'cloudfoundry',
component: CloudfoundryRollbackClusterStageConfig,
controller: 'cfRollbackClusterStageCtrl',
validators: [
{ type: 'requiredField', preventSave: true, fieldName: 'cluster' },
{ type: 'requiredField', preventSave: true, fieldName: 'regions' },
{ type: 'requiredField', preventSave: true, fieldName: 'credentials', fieldLabel: 'account' },
],
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@ import * as React from 'react';
import { mock, noop, IScope } from 'angular';
import { mount, shallow } from 'enzyme';

import { Application, ApplicationDataSource } from 'core/application';
import { IServerGroup } from 'core/domain';
import { APPLICATION_MODEL_BUILDER, ApplicationModelBuilder } from 'core/application/applicationModel.builder';
import { REACT_MODULE } from 'core/reactShims';
import {
Application,
APPLICATION_MODEL_BUILDER,
ApplicationModelBuilder,
ApplicationDataSource,
IMoniker,
IServerGroup,
REACT_MODULE,
} from '@spinnaker/core';

import { AccountRegionClusterSelector, IAccountRegionClusterSelectorProps } from './AccountRegionClusterSelector';

Expand All @@ -22,6 +27,7 @@ describe('<AccountRegionClusterSelector />', () => {
region,
instances: [{ health: null, id: 'instance-id', launchTime: 0, name: 'instance-name', zone: 'GMT' }],
instanceCounts: { up: 1, down: 0, starting: 0, succeeded: 1, failed: 0, unknown: 0, outOfService: 0 },
moniker: { app: 'my-app', cluster, detail: 'my-detail', stack: 'my-stack', sequence: 1 },
} as IServerGroup;
}

Expand Down Expand Up @@ -237,6 +243,7 @@ describe('<AccountRegionClusterSelector />', () => {

it('the cluster value is updated in the component when cluster is changed', () => {
let cluster = '';
let moniker: IMoniker = { app: '' };
const accountRegionClusterProps: IAccountRegionClusterSelectorProps = {
accounts: [
{
Expand All @@ -257,6 +264,7 @@ describe('<AccountRegionClusterSelector />', () => {
clusterField: 'newCluster',
onComponentUpdate: (value: any) => {
cluster = value.newCluster;
moniker = value.moniker;
},
component: {
cluster: 'app-stack-detailOne',
Expand All @@ -265,6 +273,14 @@ describe('<AccountRegionClusterSelector />', () => {
},
};

const expectedMoniker = {
app: 'my-app',
cluster: 'app-stack-detailThree',
detail: 'my-detail',
stack: 'my-stack',
sequence: null,
} as IMoniker;

const component = mount<AccountRegionClusterSelector>(
<AccountRegionClusterSelector {...accountRegionClusterProps} />,
);
Expand All @@ -285,6 +301,7 @@ describe('<AccountRegionClusterSelector />', () => {
$scope.$digest();

expect(cluster).toBe('app-stack-detailThree');
expect(moniker).toEqual(expectedMoniker);
});

it('initialize with form names', () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
import * as React from 'react';

import { first, isNil, uniq } from 'lodash';

import Select, { Option } from 'react-select';

import {
Application,
AppListExtractor,
IAccount,
IMoniker,
IServerGroup,
IServerGroupFilter,
StageConfigField,
Expand Down Expand Up @@ -113,10 +116,25 @@ export class AccountRegionClusterSelector extends React.Component<
};

public onClusterUpdate = (option: Option<string>): void => {
const clusterName = option.value;
const filterByCluster = AppListExtractor.monikerClusterNameFilter(clusterName);
const clusterMoniker = first(uniq(AppListExtractor.getMonikers([this.props.application], filterByCluster)));
let moniker: IMoniker;

if (isNil(clusterMoniker)) {
// remove the moniker from the stage if one doesn't exist.
moniker = undefined;
} else {
// clusters don't contain sequences, so null it out.
clusterMoniker.sequence = null;
moniker = clusterMoniker;
}

this.props.onComponentUpdate &&
this.props.onComponentUpdate({
...this.props.component,
[this.state.clusterField]: option.value,
[this.state.clusterField]: clusterName,
moniker,
});
};

Expand Down

0 comments on commit e0a8639

Please sign in to comment.