Skip to content

Commit

Permalink
fix(explicitRollback): Add configurable timeout for serverGroup looku…
Browse files Browse the repository at this point in the history
…p from Clouddriver API (#4686) (#4692)

* fix(explicitRollback): Add configurable timeout for serverGroup rollback from Clouddriver API

* fix(explicitRollback): Fix tests

* fix(explicitRollback): Add configurable timeout for serverGroup lookup from Clouddriver API

* fix(explicitRollback): Add configurable timeout for serverGroup lookup from Clouddriver API

* fix(explicitRollback): Add configurable timeout for serverGroup lookup from Clouddriver API

---------

Co-authored-by: Jason <[email protected]>
(cherry picked from commit 2b8af02)

Co-authored-by: Christos Arvanitis <[email protected]>
  • Loading branch information
mergify[bot] and christosarvanitis authored Apr 2, 2024
1 parent 4539002 commit baedae5
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import com.netflix.spinnaker.kork.core.RetrySupport
import com.netflix.spinnaker.kork.exceptions.SpinnakerException
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
import com.netflix.spinnaker.orca.clouddriver.config.RollbackConfigurationProperties
import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.ApplySourceServerGroupCapacityStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.CaptureSourceServerGroupCapacityStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.DisableServerGroupStage
Expand All @@ -34,6 +35,7 @@ import com.netflix.spinnaker.orca.api.pipeline.SyntheticStageOwner
import com.netflix.spinnaker.security.AuthenticatedRequest
import groovy.util.logging.Slf4j
import org.springframework.beans.factory.annotation.Autowired
import org.springframework.beans.factory.annotation.Value

import javax.annotation.Nullable
import java.util.concurrent.Callable
Expand All @@ -49,6 +51,10 @@ class ExplicitRollback implements Rollback {
Boolean disableOnly
Boolean enableAndDisableOnly

@Autowired
@JsonIgnore
RollbackConfigurationProperties rollbackConfigurationProperties;

@JsonIgnore
private final ExecutorService executor = java.util.concurrent.Executors.newSingleThreadExecutor()

Expand Down Expand Up @@ -149,6 +155,7 @@ class ExplicitRollback implements Rollback {

@Nullable TargetServerGroup lookupServerGroup(StageExecution parentStage, String serverGroupName) {
def fromContext = parentStage.mapTo(ResizeStrategy.Source)
def rollbackTimeout = rollbackConfigurationProperties.explicitRollback.getTimeout()

try {
// we use an executor+future to timebox how long we can spend in this remote call
Expand All @@ -163,11 +170,11 @@ class ExplicitRollback implements Rollback {
})

executor.submit(authenticatedRequest)
.get(5, TimeUnit.SECONDS)
.get(rollbackTimeout, TimeUnit.SECONDS)
.get() // not sure what would cause the Optional to not be present but we would catch and log it
} catch(Exception e) {
log.error('Could not generate resize stage because there was an error looking up {}', serverGroupName, e)
throw new SpinnakerException("failed to look up ${serverGroupName}", e)
throw new SpinnakerException("Failed Clouddriver look up for ${serverGroupName} in ${rollbackTimeout} sec. Consider increasing rollback.explicitRollback.timeout in orca profile.", e)
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright 2024 Harness, Inc.
*
* Licensed 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 com.netflix.spinnaker.orca.clouddriver.config;

import lombok.Data;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.DeprecatedConfigurationProperty;
import org.springframework.boot.context.properties.NestedConfigurationProperty;
import org.springframework.context.annotation.Configuration;

@Data
@Configuration
@ConfigurationProperties(prefix = "rollback")
public class RollbackConfigurationProperties {

private static final Logger logger =
LoggerFactory.getLogger(RollbackConfigurationProperties.class);

@Value("${rollback.timeout.enabled:false}")
private boolean dynamicRollbackEnabled;

@Deprecated
@DeprecatedConfigurationProperty(replacement = "rollback.dynamicRollback.enabled")
public boolean getDynamicRollbackEnabled() {
return dynamicRollbackEnabled;
}

@NestedConfigurationProperty private ExplicitRollback explicitRollback = new ExplicitRollback();

@NestedConfigurationProperty private DynamicRollback dynamicRollback = new DynamicRollback();

@Getter
@Setter
@NoArgsConstructor
public static class ExplicitRollback {
private int timeout = 5;
}

@Getter
@Setter
@NoArgsConstructor
public static class DynamicRollback {
private boolean enabled = false;
}

public boolean isDynamicRollbackEnabled() {
if (getDynamicRollbackEnabled()) {
logger.warn(
"The rollback.timeout.enabled property is deprecated and will be removed in a future release. Please use rollback.dynamicRollback.enabled instead.");
}
return dynamicRollback.isEnabled() || getDynamicRollbackEnabled();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution;
import com.netflix.spinnaker.orca.clouddriver.CloudDriverService;
import com.netflix.spinnaker.orca.clouddriver.FeaturesService;
import com.netflix.spinnaker.orca.clouddriver.config.RollbackConfigurationProperties;
import com.netflix.spinnaker.orca.clouddriver.model.Cluster;
import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup;
import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup.Capacity;
Expand All @@ -52,7 +53,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.stereotype.Component;

/**
Expand Down Expand Up @@ -92,13 +92,13 @@ public DetermineRollbackCandidatesTask(
RetrySupport retrySupport,
CloudDriverService cloudDriverService,
FeaturesService featuresService,
@Value("${rollback.timeout.enabled:false}") boolean dynamicRollbackTimeoutEnabled) {
RollbackConfigurationProperties rollbackConfigurationProperties) {
this.retrySupport = retrySupport;
this.cloudDriverService = cloudDriverService;
this.previousImageRollbackSupport =
new PreviousImageRollbackSupport(
objectMapper, cloudDriverService, featuresService, retrySupport);
this.dynamicRollbackTimeoutEnabled = dynamicRollbackTimeoutEnabled;
this.dynamicRollbackTimeoutEnabled = rollbackConfigurationProperties.isDynamicRollbackEnabled();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.rollback

import com.netflix.spinnaker.kork.exceptions.SpinnakerException
import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
import com.netflix.spinnaker.orca.clouddriver.config.RollbackConfigurationProperties
import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.ApplySourceServerGroupCapacityStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.providers.aws.CaptureSourceServerGroupCapacityStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.DisableServerGroupStage
Expand All @@ -42,6 +43,15 @@ class ExplicitRollbackSpec extends Specification {
def captureSourceServerGroupCapacityStage = new CaptureSourceServerGroupCapacityStage()
def applySourceServerGroupCapacityStage = new ApplySourceServerGroupCapacityStage()
def waitStage = new WaitStage()

def explicitRollback = new RollbackConfigurationProperties.ExplicitRollback(
timeout: 20
)
def rollbackConfigurationProperties = new RollbackConfigurationProperties(
explicitRollback: explicitRollback

)

CloudDriverService cloudDriverService = Mock()

def stage = stage {
Expand All @@ -61,7 +71,8 @@ class ExplicitRollbackSpec extends Specification {
captureSourceServerGroupCapacityStage: captureSourceServerGroupCapacityStage,
applySourceServerGroupCapacityStage: applySourceServerGroupCapacityStage,
waitStage: waitStage,
cloudDriverService: cloudDriverService
cloudDriverService: cloudDriverService,
rollbackConfigurationProperties: rollbackConfigurationProperties
)

def setup() {
Expand Down Expand Up @@ -213,4 +224,5 @@ class ExplicitRollbackSpec extends Specification {
1 * cloudDriverService.getTargetServerGroup(_, rollbackServerGroupName, _) >> { throw new Exception(":(") }
thrown(SpinnakerException)
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.netflix.spinnaker.moniker.Moniker
import com.netflix.spinnaker.orca.clouddriver.CloudDriverService
import com.netflix.spinnaker.orca.clouddriver.FeaturesService
import com.netflix.spinnaker.orca.clouddriver.ModelUtils
import com.netflix.spinnaker.orca.clouddriver.config.RollbackConfigurationProperties
import com.netflix.spinnaker.orca.clouddriver.model.EntityTags
import com.netflix.spinnaker.orca.clouddriver.model.ServerGroup
import spock.lang.Specification
Expand All @@ -34,6 +35,12 @@ import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.stage
class DetermineRollbackCandidatesTaskSpec extends Specification {
def objectMapper = new ObjectMapper()
def featuresService = Mock(FeaturesService)
def dynamicRollback = new RollbackConfigurationProperties.DynamicRollback(
enabled: true
)
def rollbackConfigurationProperties = new RollbackConfigurationProperties(
dynamicRollback: dynamicRollback
)
CloudDriverService cloudDriverService = Mock()

@Subject
Expand All @@ -42,7 +49,7 @@ class DetermineRollbackCandidatesTaskSpec extends Specification {
new RetrySupport(),
cloudDriverService,
featuresService,
true
rollbackConfigurationProperties
)

def stage = stage {
Expand All @@ -68,6 +75,7 @@ class DetermineRollbackCandidatesTaskSpec extends Specification {

def "should use dynamic timeout" () {
given: "The user preferred rollbackTimeout is present in the context"

stage.context["rollbackTimeout"] = 2

when: "The dynamicRollback is enabled"
Expand Down

0 comments on commit baedae5

Please sign in to comment.