Skip to content
This repository has been archived by the owner on Apr 10, 2024. It is now read-only.

feat: able to specify api server flags #46

Merged
merged 4 commits into from
Mar 26, 2023
Merged

feat: able to specify api server flags #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4de5cf2
feat: able to specify api server flags
csviri Mar 24, 2023
4fb621e
impl
csviri Mar 24, 2023
a729ac8
wip
csviri Mar 24, 2023
f1f1a03
remove test
csviri Mar 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion core/src/main/java/io/javaoperatorsdk/jenvtest/KubeAPIServerConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package io.javaoperatorsdk.jenvtest;

import java.util.List;
import java.util.Optional;

public class KubeAPIServerConfig {
Expand All @@ -20,10 +21,19 @@ public class KubeAPIServerConfig {
*/
private final boolean offlineMode;

KubeAPIServerConfig(String jenvtestDir, String apiServerVersion, boolean offlineMode) {
/**
* Flags to pass to Kube API Server on startup. Key and value are two separated items, like
* specifying min-request-timeout needs to add in order two values: "--min-request-timeout" and
* "300" for the actual desired value.
*/
private final List<String> apiServerFlags;

KubeAPIServerConfig(String jenvtestDir, String apiServerVersion, boolean offlineMode,
List<String> apiServerFlags) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it could be useful to have a Flag class that would encapsulate things instead of just relying on String pairs (similarly to what's done for micrometer tags)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh not sure, it is also some time not a pair, just a single flag name

this.jenvtestDir = jenvtestDir;
this.apiServerVersion = apiServerVersion;
this.offlineMode = offlineMode;
this.apiServerFlags = apiServerFlags;
}

public String getJenvtestDir() {
Expand All @@ -37,4 +47,8 @@ public Optional<String> getApiServerVersion() {
public boolean isOfflineMode() {
return offlineMode;
}

public List<String> getApiServerFlags() {
return apiServerFlags;
}
}
27 changes: 26 additions & 1 deletion core/src/main/java/io/javaoperatorsdk/jenvtest/KubeAPIServerConfigBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package io.javaoperatorsdk.jenvtest;

import java.io.File;
import java.util.ArrayList;
import java.util.List;

public final class KubeAPIServerConfigBuilder {

Expand All @@ -13,6 +15,7 @@ public final class KubeAPIServerConfigBuilder {
private String jenvtestDir;
private String apiServerVersion;
private Boolean offlineMode;
private final List<String> apiServerFlags = new ArrayList<>(0);

public KubeAPIServerConfigBuilder() {}

Expand Down Expand Up @@ -58,6 +61,28 @@ public KubeAPIServerConfig build() {
this.apiServerVersion = apiServerVersionEnvVar;
}
}
return new KubeAPIServerConfig(jenvtestDir, apiServerVersion, offlineMode);
return new KubeAPIServerConfig(jenvtestDir, apiServerVersion, offlineMode, apiServerFlags);
}

public void withApiServerFlags(List<String> flags) {
apiServerFlags.addAll(flags);
}

public void withApiServerFlag(String key, String value) {
checkKeyPrefix(key);
apiServerFlags.add(key);
apiServerFlags.add(value);
}

public void withApiServerFlag(String key) {
checkKeyPrefix(key);
apiServerFlags.add(key);
}

private void checkKeyPrefix(String key) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it'd be better to issue a warning and add the prefix automatically if it's missing to be more user-friendly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually yes, was thinking to ask for the flag without the flag

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a problem with the list of flags. I will merge this simple form, and we can do impovements in a subsequent PR.

if (!key.startsWith("--")) {
throw new JenvtestException(
"Kube API Server flag needs to start with double dash: '--'; Instead found key: " + key);
}
}
}
1 change: 1 addition & 0 deletions core/src/main/java/io/javaoperatorsdk/jenvtest/junit/EnableKubeAPIServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@
*/
String kubeAPIVersion() default NOT_SET;

String[] apiServerFlags() default {};
}
8 changes: 6 additions & 2 deletions core/src/main/java/io/javaoperatorsdk/jenvtest/junit/KubeAPIServerExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.lang.reflect.AnnotatedElement;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -46,11 +47,11 @@ public void afterEach(ExtensionContext extensionContext) {
private void startIfAnnotationPresent(ExtensionContext extensionContext) {
extensionContext.getElement().ifPresent(ae -> {
var annotation = getExtensionAnnotationInstance(ae);
annotation.ifPresent(a -> startApiServer(extensionContext, a));
annotation.ifPresent(this::startApiServer);
});
}

private void startApiServer(ExtensionContext context, EnableKubeAPIServer annotation) {
private void startApiServer(EnableKubeAPIServer annotation) {
kubeApiServer = new KubeAPIServer(annotationToConfig(annotation));
kubeApiServer.start();
}
Expand All @@ -69,6 +70,9 @@ private KubeAPIServerConfig annotationToConfig(EnableKubeAPIServer annotation) {
if (!NOT_SET.equals(version)) {
builder.withApiServerVersion(version);
}
if (annotation.apiServerFlags().length > 0) {
builder.withApiServerFlags(List.of(annotation.apiServerFlags()));
}
return builder.build();
}

Expand Down
43 changes: 27 additions & 16 deletions core/src/main/java/io/javaoperatorsdk/jenvtest/process/KubeAPIServerProcess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package io.javaoperatorsdk.jenvtest.process;

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;
import java.util.concurrent.atomic.AtomicBoolean;

Expand Down Expand Up @@ -39,21 +42,9 @@ public int startApiServer(int etcdPort) {
throw new JenvtestException(
"Missing binary for API Server on path: " + apiServerBinary.getAbsolutePath());
}
var port = Utils.findFreePort();
apiServerProcess = new ProcessBuilder(apiServerBinary.getAbsolutePath(),
"--cert-dir", config.getJenvtestDir(),
"--secure-port", "" + port,
"--etcd-servers", "http://0.0.0.0:" + etcdPort,
"--authorization-mode", "RBAC",
"--service-account-issuer", "https://localhost",
"--service-account-signing-key-file", certManager.getAPIServerKeyPath(),
"--service-account-signing-key-file", certManager.getAPIServerKeyPath(),
"--service-account-key-file", certManager.getAPIServerKeyPath(),
"--service-account-issuer", certManager.getAPIServerCertPath(),
"--disable-admission-plugins", "ServiceAccount",
"--client-ca-file", certManager.getClientCertPath(),
"--service-cluster-ip-range", "10.0.0.0/24",
"--allow-privileged")
var apiServerPort = Utils.findFreePort();
var command = createCommand(apiServerBinary, apiServerPort, etcdPort);
apiServerProcess = new ProcessBuilder(command)
.start();
Utils.redirectProcessOutputToLogger(apiServerProcess.getInputStream(), apiLog);
Utils.redirectProcessOutputToLogger(apiServerProcess.getErrorStream(), apiLog);
Expand All @@ -66,12 +57,32 @@ public int startApiServer(int etcdPort) {
return null;
});
log.debug("API Server started");
return port;
return apiServerPort;
} catch (IOException e) {
throw new JenvtestException(e);
}
}

private List<String> createCommand(File apiServerBinary, int apiServerPort, int etcdPort) {
var command = new ArrayList<String>();
command.add(apiServerBinary.getAbsolutePath());
command.addAll(config.getApiServerFlags());
command.addAll(List.of("--cert-dir", config.getJenvtestDir(),
"--secure-port", "" + apiServerPort,
"--etcd-servers", "http://0.0.0.0:" + etcdPort,
"--authorization-mode", "RBAC",
"--service-account-issuer", "https://localhost",
"--service-account-signing-key-file", certManager.getAPIServerKeyPath(),
"--service-account-signing-key-file", certManager.getAPIServerKeyPath(),
"--service-account-key-file", certManager.getAPIServerKeyPath(),
"--service-account-issuer", certManager.getAPIServerCertPath(),
"--disable-admission-plugins", "ServiceAccount",
"--client-ca-file", certManager.getClientCertPath(),
"--service-cluster-ip-range", "10.0.0.0/24",
"--allow-privileged"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would one capture such a parameter if you're supposed to pass pairs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is also api for that. in the builder

return command;
}

public void waitUntilDefaultNamespaceCreated() {
try {
AtomicBoolean started = new AtomicBoolean(false);
Expand Down