Skip to content

Commit

Permalink
fix(kubectl): add metrics to retries, make log messages more descript…
Browse files Browse the repository at this point in the history
…ive and support retries for all kubectl actions (#5854)

* fix(kubectl): reduce duplicate retry log messages and add retry metrics

* add tests to show duplicate logs aren't being printed when multiple threads are running the same command
* add metrics support
* refactor criteria that decides if an error should be retried. This is needed so that metrics are reported correctly
* refactor tests
* add short custom log messages for retries instead of the default verbose ones

* fix(kubectl/retries): refactor kubectl retry logic such that a new job request is created for each retry attempt

This helps prevent the input stream from being empty for calls like kubectl apply -f - on subsequent retry attempts, which leads to the 'no objects passed to apply' error message

* refactor(kubernetes/test): use MemoryAppender to simplify KubectlJobExecutorTest

Co-authored-by: Apoorv Mahajan <apoorv.mahajan@salesforce.com>
  • Loading branch information
dbyron-sf and apoorv-mahajan committed Jan 13, 2023
1 parent da953e0 commit 46706fe
Show file tree
Hide file tree
Showing 8 changed files with 1,002 additions and 358 deletions.
4 changes: 3 additions & 1 deletion clouddriver-core/clouddriver-core.gradle
Expand Up @@ -13,6 +13,9 @@ dependencies {
annotationProcessor "org.projectlombok:lombok"
testAnnotationProcessor "org.projectlombok:lombok"

// Because a JobRequest constructor takes a org.apache.commons.exec.CommandLine argument
api "org.apache.commons:commons-exec"

// This is because some classes in this module use the Groovy @Immutable annotation,
// which appears to require consumers to have core groovy on the classpath
api "org.codehaus.groovy:groovy"
Expand Down Expand Up @@ -43,7 +46,6 @@ dependencies {
implementation "com.jakewharton.retrofit:retrofit1-okhttp3-client"
implementation "io.reactivex:rxjava"
implementation "net.jodah:failsafe:1.0.4"
implementation "org.apache.commons:commons-exec"
implementation "org.codehaus.groovy:groovy"
implementation "org.codehaus.groovy:groovy-templates"
implementation "org.springframework.boot:spring-boot-actuator"
Expand Down
Expand Up @@ -18,6 +18,7 @@
import java.io.ByteArrayInputStream;
import java.io.File;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import lombok.Getter;
Expand Down Expand Up @@ -53,8 +54,8 @@ public JobRequest(List<String> tokenizedCommand, InputStream inputStream, File w
this(tokenizedCommand, System.getenv(), inputStream, workingDir);
}

public JobRequest(List<String> tokenizedCommand, File workingDor) {
this(tokenizedCommand, System.getenv(), new ByteArrayInputStream(new byte[0]), workingDor);
public JobRequest(List<String> tokenizedCommand, File workingDir) {
this(tokenizedCommand, System.getenv(), new ByteArrayInputStream(new byte[0]), workingDir);
}

public JobRequest(
Expand All @@ -69,6 +70,15 @@ public JobRequest(
this.workingDir = workingDir;
}

// only used in tests
public JobRequest(CommandLine commandLine, InputStream inputStream) {
this.tokenizedCommand = new ArrayList<>();
this.commandLine = commandLine;
this.environment = System.getenv();
this.inputStream = inputStream;
this.workingDir = null;
}

private CommandLine createCommandLine(List<String> tokenizedCommand) {
if (tokenizedCommand == null || tokenizedCommand.size() == 0) {
throw new IllegalArgumentException("No tokenizedCommand specified.");
Expand All @@ -85,6 +95,9 @@ private CommandLine createCommandLine(List<String> tokenizedCommand) {

@Override
public String toString() {
return String.join(" ", tokenizedCommand);
if (!tokenizedCommand.isEmpty()) {
return String.join(" ", tokenizedCommand);
}
return commandLine.toString();
}
}
3 changes: 3 additions & 0 deletions clouddriver-kubernetes/clouddriver-kubernetes.gradle
Expand Up @@ -88,7 +88,10 @@ dependencies {
implementation "org.springframework.cloud:spring-cloud-context"
implementation "org.springframework.cloud:spring-cloud-config-server"
implementation "io.github.resilience4j:resilience4j-retry"
implementation "io.github.resilience4j:resilience4j-micrometer"

testImplementation "io.spinnaker.kork:kork-test"
testImplementation "org.apache.commons:commons-exec"
testImplementation "org.assertj:assertj-core"
testImplementation "org.junit.jupiter:junit-jupiter-api"
testImplementation "org.junit.jupiter:junit-jupiter-params"
Expand Down
Expand Up @@ -35,6 +35,9 @@ public class KubernetesConfigurationProperties {

private Cache cache = new Cache();

private KubectlProperties kubectl = new KubectlProperties();
private OAuthProperties oAuth = new OAuthProperties();

public KubernetesConfigurationProperties kubernetesConfigurationProperties() {
return new KubernetesConfigurationProperties();
}
Expand Down Expand Up @@ -69,6 +72,14 @@ public static class Retries {

// only applicable when exponentialBackoff = true
long exponentialBackOffIntervalMs = 10000;

private Metrics metrics = new Metrics();

@Data
public static class Metrics {
// flag to capture retry metrics. Turned off by default
private boolean enabled;
}
}
}

Expand Down Expand Up @@ -120,4 +131,16 @@ public static class Cache {
*/
boolean checkApplicationInFront50 = false;
}

/** kubectl configuration properties */
@Data
public static class KubectlProperties {
private String executable = "kubectl";
}

/** oAuth configuration properties */
@Data
public static class OAuthProperties {
private String executable = "oauth2l";
}
}

0 comments on commit 46706fe

Please sign in to comment.