Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

052 server side apply #9306

Closed
wants to merge 33 commits into from
Closed
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0e8f19b
added jenvtest to test server side apply on the pod operator
jamesfarlow Oct 27, 2023
6f9f929
created a new test class for the podoperator tests when using server …
jamesfarlow Oct 30, 2023
c943da0
add pod operator server side apply tests
jamesfarlow Oct 30, 2023
ce59f87
tidied up comments
jamesfarlow Oct 31, 2023
cfa1860
set log level back to INFO
jamesfarlow Oct 31, 2023
63887dc
pass boolean value for feature gate to ResourceOperatorSupplier
jamesfarlow Nov 1, 2023
1563b9e
add javadoc to ResourceOperatorSupplier
jamesfarlow Nov 1, 2023
7a0da73
log exception
jamesfarlow Nov 1, 2023
ce2ad64
added ServerSideApply to StrimziPodSetOperator and added tests
jamesfarlow Nov 1, 2023
4876486
add +UseServerSideApply to regression tests
jamesfarlow Nov 9, 2023
07c9f00
add useServerSideApply to all operators used in ResourceOperatorSupplier
jamesfarlow Nov 14, 2023
26b4441
Update jenvtest
adamj-m Jan 12, 2024
1778470
Fix import order in PodOperatorServerSideApplyTest to make checkstyle…
adamj-m Jan 12, 2024
728bce9
Fix StrimziPodSet import path in StrimziPodSetCrdOperatorServerSideAp…
adamj-m Feb 1, 2024
9606961
Do not rely on "instanceof ReconcileResult.Patched" for triggering ce…
adamj-m Feb 1, 2024
af7c416
Maybe try to execute rolling update if reconciled using server side a…
adamj-m Feb 1, 2024
e191b5a
Nullify managedFields before patching resource when server side apply…
adamj-m Feb 1, 2024
da081a0
Reset strimzi.io/manual-rolling-update anotation to prevent rolling p…
adamj-m Feb 1, 2024
9aa0068
Set default value for strimzi.io/force-renew annotation to prevent lo…
adamj-m Feb 1, 2024
2db4c7f
Add usingServerSideApply field to know whether apply was done using SSA
adamj-m Feb 1, 2024
6fe51ab
Fix race conditions in UserControllerMockTest
adamj-m Feb 1, 2024
ee938c8
Make checkstyle happy
adamj-m Feb 1, 2024
eaf65c7
Bump jenvtest version
adamj-m Feb 1, 2024
766d4d5
Handle case when patch result is null - mostly in tests
adamj-m Feb 1, 2024
d23dd74
Remove jenvtest and use MockKube3 for SSA test
adamj-m Feb 27, 2024
b4acc12
Add --overwrite for annotating secrets
adamj-m Mar 7, 2024
8bcb9fc
Do not add empty string as an argument when annotating secret
adamj-m Mar 12, 2024
a7e32db
Add info about SSA to changelog
adamj-m Mar 21, 2024
b54e14f
Add feature gates to user operator
adamj-m Mar 29, 2024
61930da
Add Cluster Operator mock tests with the SSA feature gate enabled
adamj-m Mar 29, 2024
09487c0
Add SSA support to io.strimzi.operator.common.operator.resource.concu…
adamj-m Mar 29, 2024
c0422bf
Make checkstyle happy
adamj-m Mar 29, 2024
6ac2161
Refactor of tests and adding a couple of mockito ones
krystian-kulgawczuk-natwest Jun 27, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ jobs:
display_name: 'feature-gates-regression-bundle I. - kafka + oauth'
profile: 'azp_kafka_oauth'
cluster_operator_install_type: 'bundle'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
timeout: 360
releaseVersion: '${{ parameters.releaseVersion }}'
Expand All @@ -17,7 +17,7 @@ jobs:
display_name: 'feature-gates-regression-bundle II. - security'
profile: 'azp_security'
cluster_operator_install_type: 'bundle'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
timeout: 360
releaseVersion: '${{ parameters.releaseVersion }}'
Expand All @@ -29,7 +29,7 @@ jobs:
display_name: 'feature-gates-regression-bundle III. - dynconfig + tracing + watcher'
profile: 'azp_dynconfig_listeners_tracing_watcher'
cluster_operator_install_type: 'bundle'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
timeout: 360
releaseVersion: '${{ parameters.releaseVersion }}'
Expand All @@ -41,7 +41,7 @@ jobs:
display_name: 'feature-gates-regression-bundle IV. - operators'
profile: 'azp_operators'
cluster_operator_install_type: 'bundle'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
timeout: 360
releaseVersion: '${{ parameters.releaseVersion }}'
Expand All @@ -53,7 +53,7 @@ jobs:
display_name: 'feature-gates-regression-bundle V. - rollingupdate'
profile: 'azp_rolling_update_bridge'
cluster_operator_install_type: 'bundle'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
timeout: 360
releaseVersion: '${{ parameters.releaseVersion }}'
Expand All @@ -65,7 +65,7 @@ jobs:
display_name: 'feature-gates-regression-bundle VI. - connect + mirrormaker'
profile: 'azp_connect_mirrormaker'
cluster_operator_install_type: 'bundle'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
timeout: 360
releaseVersion: '${{ parameters.releaseVersion }}'
Expand All @@ -77,7 +77,7 @@ jobs:
display_name: 'feature-gates-regression-bundle VII. - remaining system tests'
profile: 'azp_remaining'
cluster_operator_install_type: 'bundle'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
timeout: 360
releaseVersion: '${{ parameters.releaseVersion }}'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
cluster_operator_install_type: 'bundle'
timeout: 360
strimzi_rbac_scope: NAMESPACE
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
releaseVersion: '${{ parameters.releaseVersion }}'
kafkaVersion: '${{ parameters.kafkaVersion }}'
Expand All @@ -22,7 +22,7 @@ jobs:
cluster_operator_install_type: 'bundle'
timeout: 360
strimzi_rbac_scope: NAMESPACE
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
releaseVersion: '${{ parameters.releaseVersion }}'
kafkaVersion: '${{ parameters.kafkaVersion }}'
Expand All @@ -36,7 +36,7 @@ jobs:
cluster_operator_install_type: 'bundle'
timeout: 360
strimzi_rbac_scope: NAMESPACE
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
releaseVersion: '${{ parameters.releaseVersion }}'
kafkaVersion: '${{ parameters.kafkaVersion }}'
Expand All @@ -50,7 +50,7 @@ jobs:
cluster_operator_install_type: 'bundle'
timeout: 360
strimzi_rbac_scope: NAMESPACE
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
releaseVersion: '${{ parameters.releaseVersion }}'
kafkaVersion: '${{ parameters.kafkaVersion }}'
Expand All @@ -64,7 +64,7 @@ jobs:
cluster_operator_install_type: 'bundle'
timeout: 360
strimzi_rbac_scope: NAMESPACE
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
releaseVersion: '${{ parameters.releaseVersion }}'
kafkaVersion: '${{ parameters.kafkaVersion }}'
Expand All @@ -78,7 +78,7 @@ jobs:
cluster_operator_install_type: 'bundle'
timeout: 360
strimzi_rbac_scope: NAMESPACE
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
releaseVersion: '${{ parameters.releaseVersion }}'
kafkaVersion: '${{ parameters.kafkaVersion }}'
Expand All @@ -92,7 +92,7 @@ jobs:
cluster_operator_install_type: 'bundle'
timeout: 360
strimzi_rbac_scope: NAMESPACE
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft'
strimzi_feature_gates: '-UnidirectionalTopicOperator,-UseKRaft,+UseServerSideApply'
strimzi_use_node_pools_in_tests: "false"
releaseVersion: '${{ parameters.releaseVersion }}'
kafkaVersion: '${{ parameters.kafkaVersion }}'
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* The `KafkaNodePools` feature gate moves to GA stage and is permanently enabled without the possibility to disable it.
To use the Kafka Node Pool resources, you still need to use the `strimzi.io/node-pools: enabled` annotation on the `Kafka` custom resources.
* Added support for configuring the `externalIPs` field in node port type services.
* Added support for [Server-Side Apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) via `UseServerSideApply` feature gate. The feature is disabled by default.
If needed, `UseServerSideApply` can be enabled in the feature gates configuration in the Cluster Operator.

## 0.40.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
*/
package io.strimzi.operator.cluster;

import io.strimzi.operator.common.InvalidConfigurationException;
import io.strimzi.operator.common.config.FeatureGate;
import io.strimzi.operator.common.config.FeatureGatesParser;

import java.util.List;

import static java.util.Arrays.asList;
import java.util.Map;
import java.util.stream.Collectors;

/**
* Class for handling the configuration of feature gates
Expand All @@ -18,82 +19,42 @@ public class FeatureGates {

private static final String USE_KRAFT = "UseKRaft";
private static final String UNIDIRECTIONAL_TOPIC_OPERATOR = "UnidirectionalTopicOperator";
private static final String USE_SERVER_SIDE_APPLY = "UseServerSideApply";

// When adding new feature gates, do not forget to add them to allFeatureGates() and toString() methods
private final FeatureGate useKRaft = new FeatureGate(USE_KRAFT, true);
private final FeatureGate unidirectionalTopicOperator = new FeatureGate(UNIDIRECTIONAL_TOPIC_OPERATOR, true);
private final Map<String, FeatureGate> featureGates = Map.ofEntries(
Map.entry(USE_KRAFT, new FeatureGate(USE_KRAFT, true)),
Map.entry(UNIDIRECTIONAL_TOPIC_OPERATOR, new FeatureGate(UNIDIRECTIONAL_TOPIC_OPERATOR, true)),
Map.entry(USE_SERVER_SIDE_APPLY, new FeatureGate(USE_SERVER_SIDE_APPLY, false))
);

/**
* Constructs the feature gates configuration.
*
* @param featureGateConfig String with comma separated list of enabled or disabled feature gates
*/
public FeatureGates(String featureGateConfig) {
if (featureGateConfig != null && !featureGateConfig.trim().isEmpty()) {
List<String> featureGates;

if (featureGateConfig.matches("(\\s*[+-][a-zA-Z0-9]+\\s*,)*\\s*[+-][a-zA-Z0-9]+\\s*")) {
featureGates = asList(featureGateConfig.trim().split("\\s*,+\\s*"));
} else {
throw new InvalidConfigurationException(featureGateConfig + " is not a valid feature gate configuration");
}

for (String featureGate : featureGates) {
boolean value = '+' == featureGate.charAt(0);
featureGate = featureGate.substring(1);

switch (featureGate) {
case USE_KRAFT:
setValueOnlyOnce(useKRaft, value);
break;
case UNIDIRECTIONAL_TOPIC_OPERATOR:
setValueOnlyOnce(unidirectionalTopicOperator, value);
break;
default:
throw new InvalidConfigurationException("Unknown feature gate " + featureGate + " found in the configuration");
}
}

validateInterDependencies();
}
}

/**
* Validates any dependencies between various feature gates. When the dependencies are not satisfied,
* InvalidConfigurationException is thrown.
*/
private void validateInterDependencies() {
// There are currently no interdependencies between different feature gates.
// But we keep this method as these might happen again in the future.
}

/**
* Sets the feature gate value if it was not set yet. But if it is already set, then it throws an exception. This
* helps to ensure that each feature gate is configured always only once.
*
* @param gate Feature gate which is being configured
* @param value Value which should be set
*/
private void setValueOnlyOnce(FeatureGate gate, boolean value) {
if (gate.isSet()) {
throw new InvalidConfigurationException("Feature gate " + gate.getName() + " is configured multiple times");
}

gate.setValue(value);
new FeatureGatesParser(featureGateConfig).applyFor(featureGates);
}

/**
* @return Returns true when the UseKRaft feature gate is enabled
*/
public boolean useKRaftEnabled() {
return useKRaft.isEnabled();
return featureGates.get(USE_KRAFT).isEnabled();
}

/**
* @return Returns true when the UnidirectionalTopicOperator feature gate is enabled
*/
public boolean unidirectionalTopicOperatorEnabled() {
return unidirectionalTopicOperator.isEnabled();
return featureGates.get(UNIDIRECTIONAL_TOPIC_OPERATOR).isEnabled();
}

/**
* @return Returns true when the UseServerSideApply feature gate is enabled
*/
public boolean useServerSideApply() {
return featureGates.get(USE_SERVER_SIDE_APPLY).isEnabled();
}

/**
Expand All @@ -102,74 +63,15 @@ public boolean unidirectionalTopicOperatorEnabled() {
* @return List of all Feature Gates
*/
/*test*/ List<FeatureGate> allFeatureGates() {
return List.of(
useKRaft,
unidirectionalTopicOperator
);
return featureGates.values().stream().toList();
}

@Override
public String toString() {
return "FeatureGates(" +
"UseKRaft=" + useKRaft.isEnabled() + "," +
"UnidirectionalTopicOperator=" + unidirectionalTopicOperator.isEnabled() +
")";
}

/**
* Feature gate class represents individual feature fate
*/
static class FeatureGate {
private final String name;
private final boolean defaultValue;
private Boolean value = null;

/**
* Feature fate constructor
*
* @param name Name of the feature gate
* @param defaultValue Default value of the feature gate
*/
FeatureGate(String name, boolean defaultValue) {
this.name = name;
this.defaultValue = defaultValue;
}

/**
* @return The name of the feature gate
*/
public String getName() {
return name;
}

/**
* @return Returns true if the value for this feature gate is already set or false if it is still null
*/
public boolean isSet() {
return value != null;
}

/**
* Sets the value of the feature gate
*
* @param value Value of the feature gate
*/
public void setValue(boolean value) {
this.value = value;
}

/**
* @return True if the feature gate is enabled. False otherwise.
*/
public boolean isEnabled() {
return value == null ? defaultValue : value;
}

/**
* @return Returns True if this feature gate is enabled by default. False otherwise.
*/
public boolean isEnabledByDefault() {
return defaultValue;
}
String featureGatesValues = featureGates.entrySet()
.stream()
.map(featureGate -> featureGate.getKey() + "=" + featureGate.getValue().isEnabled())
.collect(Collectors.joining(","));
return "FeatureGates(%s)".formatted(featureGatesValues);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ static CompositeFuture deployClusterOperatorVerticles(Vertx vertx, KubernetesCli
metricsProvider,
pfa,
config.getOperationTimeoutMs(),
config.getOperatorName()
config.getOperatorName(),
config.featureGates().useServerSideApply()
);

// Initialize the PodSecurityProvider factory to provide the user configured provider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,19 @@ public static CertAndKey keyStoreCertAndKey(Secret secret, String keyCertName) {
* changed. This method is used to evaluate whether rolling update of existing brokers is needed when secrets with
* certificates change. It separates changes for existing certificates with other changes to the Secret such as
* added or removed certificates (scale-up or scale-down).
* <p>
* Note: this method checks if secrets differ, not whether secret is being created for first time.
*
* @param current Existing secret
* @param desired Desired secret
*
* @return True if there is a key which exists in the data sections of both secrets and which changed.
*/
public static boolean doExistingCertificatesDiffer(Secret current, Secret desired) {
if (current == null || desired == null) {
return false;
}

Comment on lines +188 to +191
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed? If it really is needed, maybe you could add some explanatory comment?

Map<String, String> currentData = current.getData();
Map<String, String> desiredData = desired.getData();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,10 @@ private Map<String, String> preparePodSetAnnotations(Storage storage) {
controllerAnnotations.put(ANNO_STRIMZI_IO_KAFKA_VERSION, kafkaVersion.version());
controllerAnnotations.put(Annotations.ANNO_STRIMZI_IO_STORAGE, ModelUtils.encodeStorageToJson(storage));

//Take ownership of the rolling update annotation (for SSA) and reset it
//as (potential) one-time roll was already ran by reconcile loop run if it was needed.
controllerAnnotations.put(Annotations.ANNO_STRIMZI_IO_MANUAL_ROLLING_UPDATE, "false");

scholzj marked this conversation as resolved.
Show resolved Hide resolved
return controllerAnnotations;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,12 @@ public StrimziPodSet generatePodSet(int replicas,
ownerReference,
templatePodSet,
replicas,
Map.of(Annotations.ANNO_STRIMZI_IO_STORAGE, ModelUtils.encodeStorageToJson(storage)),
Map.of(
Annotations.ANNO_STRIMZI_IO_STORAGE, ModelUtils.encodeStorageToJson(storage),
//Take ownership of the rolling update annotation (for SSA) and reset it
//as (potential) one-time roll was already ran by reconcile loop run if it was needed.
Annotations.ANNO_STRIMZI_IO_MANUAL_ROLLING_UPDATE, "false"
scholzj marked this conversation as resolved.
Show resolved Hide resolved
),
labels.strimziSelectorLabels(),
podNum -> WorkloadUtils.createStatefulPod(
reconciliation,
Expand Down
Loading