Skip to content

ensures support for cluster-wide monitoring #22821

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.

Sign up for GitHub

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

Merged
vmuzikar merged 1 commit into from
Sep 19, 2023
Merged

ensures support for cluster-wide monitoring #22821

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
37 changes: 32 additions & 5 deletions .github/workflows/operator-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,41 @@ jobs:
- name: Deploy an example Keycloak and wait for it to be ready
working-directory: operator
run: |
kubectl apply -f src/main/resources/example-postgres.yaml
kubectl apply -f src/test/resources/example-postgres.yaml
./scripts/check-crds-installed.sh
kubectl apply -f src/main/resources/example-db-secret.yaml
kubectl apply -f src/main/resources/example-tls-secret.yaml
kubectl apply -f src/main/resources/example-keycloak.yaml
kubectl apply -f src/main/resources/example-realm.yaml
kubectl apply -f src/test/resources/example-db-secret.yaml
kubectl apply -f src/test/resources/example-tls-secret.yaml
kubectl apply -f src/test/resources/example-keycloak.yaml
kubectl apply -f src/test/resources/example-realm.yaml
# Wait for the CRs to be ready
./scripts/check-examples-installed.sh

- name: Single namespace cleanup
working-directory: operator
run: |
kubectl delete -f src/test/resources/example-postgres.yaml
kubectl delete -f src/test/resources/example-db-secret.yaml
kubectl delete -f src/test/resources/example-tls-secret.yaml
kubectl delete -f src/test/resources/example-keycloak.yaml
kubectl delete -f src/test/resources/example-realm.yaml

- name: Arrange OLM test installation for all namespaces
working-directory: operator
run: |
kubectl patch csv keycloak-operator.v86400000.0.0 --type merge --patch '{"spec": {"installModes": [{"type": "AllNamespaces","supported": true}]}}'
kubectl patch operatorgroup og --type json --patch '[{"op":"remove","path":"/spec/targetNamespaces"}]'

- name: Deploy an example Keycloak in a different namespace and wait for it to be ready
working-directory: operator
run: |
kubectl create ns keycloak
kubectl apply -f src/test/resources/example-postgres.yaml -n keycloak
kubectl apply -f src/test/resources/example-db-secret.yaml -n keycloak
kubectl apply -f src/test/resources/example-tls-secret.yaml -n keycloak
kubectl apply -f src/test/resources/example-keycloak.yaml -n keycloak
kubectl apply -f src/test/resources/example-realm.yaml -n keycloak
# Wait for the CRs to be ready
./scripts/check-examples-installed.sh keycloak

check:
name: Status Check - Keycloak Operator CI
Expand Down
6 changes: 4 additions & 2 deletions operator/scripts/check-examples-installed.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
#! /bin/bash
set -euxo pipefail

NAMESPACE=${1:-default}

max_retries=500
c=0
while [[ $(kubectl get keycloaks/example-kc -o jsonpath="{.status.conditions[?(@.type == 'Ready')].status}") != "True" ]]
while [[ $(kubectl -n $NAMESPACE get keycloaks/example-kc -o jsonpath="{.status.conditions[?(@.type == 'Ready')].status}") != "True" ]]
do
echo "waiting for Keycloak example-kc status"
((c++)) && ((c==max_retries)) && exit -1
sleep 1
done

c=0
while [[ $(kubectl get keycloakrealmimports/example-count0-kc -o jsonpath="{.status.conditions[?(@.type == 'Done')].status}") != "True" ]]
while [[ $(kubectl -n $NAMESPACE get keycloakrealmimports/example-count0-kc -o jsonpath="{.status.conditions[?(@.type == 'Done')].status}") != "True" ]]
do
echo "waiting for Keycloak Realm Import example-count0-kc status"
((c++)) && ((c==max_retries)) && exit -1
Expand Down
19 changes: 13 additions & 6 deletions operator/scripts/create-olm-test-resources.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ DOCKER_REGISTRY=$2

UUID=${3:-""}

TARGET_NAMESPACES=${4-default}

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

rm -rf $SCRIPT_DIR/../olm/testing-resources
Expand All @@ -27,23 +29,28 @@ spec:
interval: 10m
EOF

cat << EOF >> $SCRIPT_DIR/../olm/testing-resources/operatorgroup.yaml

OPERATOR_GROUP_FILE=$SCRIPT_DIR/../olm/testing-resources/operatorgroup.yaml

cat << EOF >> $OPERATOR_GROUP_FILE
kind: OperatorGroup
apiVersion: operators.coreos.com/v1
metadata:
name: og-single
namespace: default
name: og
spec:
targetNamespaces:
- default
EOF

IFS=', ' read -r -a array <<< "$TARGET_NAMESPACES"
for element in "${array[@]}"
do
yq ea -i ".spec.targetNamespaces += [\"$element\"]" $OPERATOR_GROUP_FILE

@vmuzikar vmuzikar Sep 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

spec.TargetNamespaces seems to be kinda deprecated? But maybe we don't even need it (specifying multiple namespaces here), see my comment above.

@shawkins shawkins Sep 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would be in favor of separating that off as a another issue around supporting the selector - in general that will be a problem as the sdk nor our logic is honoring that.

@shawkins shawkins Sep 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

olm does manage the watching of namespaces for the selector, and updates the operator deployment from there.

I see two issues with that (not related to keycloak):

  • the docuemtation is wrong, you cannot directly specify the label under the selector, it needs to be under a matchLabels stanza - if you try it as documented it does end up watching all namespaces. I'll open something for that.
  • if nothing matches the label olm won't update the deployment because it can't distinguish between watching nothing and watching everything. This seems like odd behavior as you'll end up still watching one or more namespaces that you don't intend to.

So on the script changes if you want I can either backout everything and we won't bother with those changes for now, or it could be updated to be based upon a label, or a label and target namespaces (such that the script adds the label to the targets).

@vmuzikar vmuzikar Sep 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for double checking!

So on the script changes if you want I can either backout everything and we won't bother with those changes for now, or it could be updated to be based upon a label, or a label and target namespaces (such that the script adds the label to the targets).

I'd keep only what's strictly necessary for testing these two scenarios in the CI:

  • Operator watching and being installed in a single namespace.
  • Operator watching all namespaces, i.e. empty watched namespaces.
  • And maybe one more scenario that we currently miss. That the Operator can watch a list of namespaces (MultiNamespace mode). I think we technically "support" this now. But we could add this test scenario later as a follow-up.

Which means we can keep everything in the scripts as we'd use targetNamespaces for the last scenario.

@shawkins shawkins Sep 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Which means we can keep everything in the scripts as we'd use targetNamespaces for the last scenario.

If I understand you correctly you mean just leave it as is in the pr and don't try to switch to a label or namespace/label based mechanism.

@vmuzikar vmuzikar Sep 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

don't try to switch to a label or namespace/label based mechanism.

Yeah, I'd say it's OLM responsibility. No need to test it on our side.

done

cat << EOF >> $SCRIPT_DIR/../olm/testing-resources/subscription.yaml
apiVersion: operators.coreos.com/v1alpha1
kind: Subscription
metadata:
name: keycloak-operator
namespace: default
spec:
installPlanApproval: Automatic
name: keycloak-operator
Expand Down
6 changes: 4 additions & 2 deletions operator/scripts/install-keycloak-operator.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ set -euxo pipefail

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

INSTALL_NAMESPACE=${1:-default}

# Delete the default catalog if it exists
sh -c "kubectl delete catalogsources operatorhubio-catalog -n olm | true"

Expand All @@ -18,5 +20,5 @@ do
sleep 1
done

kubectl apply -f $SCRIPT_DIR/../olm/testing-resources/operatorgroup.yaml
kubectl apply -f $SCRIPT_DIR/../olm/testing-resources/subscription.yaml
kubectl apply -f $SCRIPT_DIR/../olm/testing-resources/operatorgroup.yaml -n $INSTALL_NAMESPACE
kubectl apply -f $SCRIPT_DIR/../olm/testing-resources/subscription.yaml -n $INSTALL_NAMESPACE
8 changes: 6 additions & 2 deletions operator/scripts/olm-testing.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ set -euxo pipefail

UUID=${1:-$(git rev-parse --short HEAD)}

INSTALL_NAMESPACE=${2:-default}

TARGET_NAMESPACES=${3-$INSTALL_NAMESPACE}
Comment thread
vmuzikar marked this conversation as resolved.

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )

# This version translates to one day for ttl.sh
Expand All @@ -28,6 +32,6 @@ VERSION="86400000.0.0"
docker push "ttl.sh/${UUID}keycloak-operator:${VERSION}"
)

$SCRIPT_DIR/prepare-olm-test.sh ttl.sh ${VERSION} NONE ${UUID}
$SCRIPT_DIR/prepare-olm-test.sh ttl.sh ${VERSION} NONE ${UUID} $TARGET_NAMESPACES

$SCRIPT_DIR/install-keycloak-operator.sh
$SCRIPT_DIR/install-keycloak-operator.sh $INSTALL_NAMESPACE
4 changes: 3 additions & 1 deletion operator/scripts/prepare-olm-test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ PREV_VERSION="$3"

UUID=${4:-""}

TARGET_NAMESPACES=${5-default}

OPERATOR_IMAGE_NAME="keycloak-operator"
OPERATOR_DOCKER_IMAGE="$DOCKER_REGISTRY/${UUID}$OPERATOR_IMAGE_NAME"

Expand All @@ -31,4 +33,4 @@ $SCRIPT_DIR/create-olm-test-catalog.sh $VERSION $DOCKER_REGISTRY/${UUID}keycloak
docker push $DOCKER_REGISTRY/${UUID}keycloak-test-catalog:$VERSION)

# Create testing resources
$SCRIPT_DIR/create-olm-test-resources.sh $VERSION $DOCKER_REGISTRY ${UUID}
$SCRIPT_DIR/create-olm-test-resources.sh $VERSION $DOCKER_REGISTRY ${UUID} $TARGET_NAMESPACES
10 changes: 4 additions & 6 deletions operator/src/main/java/org/keycloak/operator/controllers/KeycloakController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@

import jakarta.inject.Inject;

import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE;

@ControllerConfiguration(namespaces = WATCH_CURRENT_NAMESPACE,
@ControllerConfiguration(
dependents = {
@Dependent(type = KeycloakAdminSecretDependentResource.class),
@Dependent(type = KeycloakIngressDependentResource.class, reconcilePrecondition = KeycloakIngressDependentResource.EnabledCondition.class),
Expand All @@ -68,20 +66,20 @@ public class KeycloakController implements Reconciler<Keycloak>, EventSourceInit

@Override
public Map<String, EventSource> prepareEventSources(EventSourceContext<Keycloak> context) {
String namespace = context.getControllerConfiguration().getConfigurationService().getKubernetesClient().getNamespace();
var namespaces = context.getControllerConfiguration().getNamespaces();

InformerConfiguration<StatefulSet> statefulSetIC = InformerConfiguration
.from(StatefulSet.class)
.withLabelSelector(Constants.DEFAULT_LABELS_AS_STRING)
.withNamespaces(namespace)
.withNamespaces(namespaces)
.withSecondaryToPrimaryMapper(Mappers.fromOwnerReference())
.withOnUpdateFilter(new MetadataAwareOnUpdateFilter<>())
.build();

InformerConfiguration<Service> servicesIC = InformerConfiguration
.from(Service.class)
.withLabelSelector(Constants.DEFAULT_LABELS_AS_STRING)
.withNamespaces(namespace)
.withNamespaces(namespaces)
.withSecondaryToPrimaryMapper(Mappers.fromOwnerReference())
.withOnUpdateFilter(new MetadataAwareOnUpdateFilter<>())
.build();
Expand Down
4 changes: 1 addition & 3 deletions operator/src/main/java/org/keycloak/operator/controllers/KeycloakRealmImportController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@

import jakarta.inject.Inject;

import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE;

@ControllerConfiguration(namespaces = WATCH_CURRENT_NAMESPACE,
@ControllerConfiguration(
dependents = {
@Dependent(type = KeycloakRealmImportSecretDependentResource.class)
})
Expand Down
4 changes: 1 addition & 3 deletions operator/src/main/java/org/keycloak/operator/controllers/WatchedSecretsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@
import jakarta.enterprise.context.ApplicationScoped;
import jakarta.inject.Inject;

import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE;

@ApplicationScoped
@ControllerConfiguration(namespaces = WATCH_CURRENT_NAMESPACE, labelSelector = Constants.KEYCLOAK_COMPONENT_LABEL + "=" + WatchedSecrets.WATCHED_SECRETS_LABEL_VALUE)
@ControllerConfiguration(labelSelector = Constants.KEYCLOAK_COMPONENT_LABEL + "=" + WatchedSecrets.WATCHED_SECRETS_LABEL_VALUE)
public class WatchedSecretsController implements Reconciler<Secret>, EventSourceInitializer<Secret>, WatchedSecrets {

@Inject
Expand Down
3 changes: 3 additions & 0 deletions operator/src/main/resources/application.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,6 @@ quarkus.openshift.env.vars.operator-keycloak-image=${operator.keycloak.image}
# Bundle config
quarkus.operator-sdk.bundle.package-name=keycloak-operator
quarkus.operator-sdk.bundle.channels=fast

quarkus.operator-sdk.namespaces=JOSDK_WATCH_CURRENT
quarkus.operator-sdk.generate-with-watched-namespaces=JOSDK_WATCH_CURRENT
2 changes: 1 addition & 1 deletion operator/src/test/java/org/keycloak/operator/testsuite/integration/BaseOperatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ private static void registerReconcilers() {

for (Reconciler<?> reconciler : reconcilers) {
Log.info("Register and apply : " + reconciler.getClass().getName());
operator.register(reconciler);
operator.register(reconciler, overrider -> overrider.settingNamespace(namespace));
}
}

Expand Down
0 ...src/main/resources/example-db-secret.yaml → ...src/test/resources/example-db-secret.yaml
View file
Open in desktop
File renamed without changes.
0 .../src/main/resources/example-keycloak.yaml → .../src/test/resources/example-keycloak.yaml
View file
Open in desktop
File renamed without changes.
0 .../src/main/resources/example-postgres.yaml → .../src/test/resources/example-postgres.yaml
View file
Open in desktop
File renamed without changes.
0 ...tor/src/main/resources/example-realm.yaml → ...tor/src/test/resources/example-realm.yaml
View file
Open in desktop
File renamed without changes.
0 ...rc/main/resources/example-tls-secret.yaml → ...rc/test/resources/example-tls-secret.yaml
View file
Open in desktop
File renamed without changes.