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

Add namespace configuration for generated manifests #651

Closed

Conversation

Jamstah
Copy link
Contributor

@Jamstah Jamstah commented Jul 10, 2023

This enables the manifests generated at build time to be customised with namespaces to watch without affecting the controllers themselves. This way the controllers can use default behaviour allowing them to be configured at runtime, and the manifests can be used to apply that configuration at runtime.

Unfortunately, the logic to test for all namespaces and current namespaces in the Java operator SDK is in protected static methods, so I had to copy that logic into this project. It may be sensible to suggest that logic is made public in the Java operator SDK instead.

This came out of work being done for the keycloak oeprator here: keycloak/keycloak#21231

This enables the manifests generated at build time to be customised with namespaces to watch without affecting the controllers themselves. This way the controllers can use default behaviour allowing them to be configured at runtime, and the manifests can be used to apply that configuration at runtime.

Unfortunately, the logic to test for all namespaces and current namespaces in the Java operator SDK is in protected static methods, so I had to copy that logic into this project. It may be sensible to suggest that logic is made public in the Java operator SDK instead.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>
@metacosm
Copy link
Member

Hi @Jamstah, unless I'm misunderstanding your issue, I think that the quarkus.operator-sdk.controllers.<controller name>.generate-with-watched-namespaces property is what you're looking for. See https://github.com/quarkiverse/quarkus-operator-sdk/releases/tag/6.2.1 (though this feature was available in 6.2.0 already).

@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 10, 2023

More likely to be my misunderstanding that yours :)


If I use this to build (new configuration from this PR):

mvn clean package -Doperator -Dquarkus.container-image.build=true -Dquarkus.operator-sdk.manifest.generate-with-watched-namespaces=JOSDK_WATCH_CURRENT

And run with the env var:

QUARKUS_OPERATOR_SDK_NAMESPACES=""

I get this in the pod logs:

2023-07-10 20:11:26,012 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakrealmimportcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.realmimport.KeycloakRealmImport' for namespace(s): [all namespaces]
2023-07-10 20:11:26,019 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.deployment.Keycloak' for namespace(s): [all namespaces]

So at runtime, I successfully overrode the namespaces.


If I use this to build:

mvn clean package -Doperator -Dquarkus.container-image.build=true -Dquarkus.operator-sdk.controllers.keycloakcontroller.generate-with-watched-namespaces=jammy

And run with the env var:

QUARKUS_OPERATOR_SDK_NAMESPACES=""

I get this in the pod logs:

2023-07-10 20:20:27,219 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakrealmimportcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.realmimport.KeycloakRealmImport' for namespace(s): [all namespaces]
2023-07-10 20:20:27,253 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.deployment.Keycloak' for namespace(s): [jammy]

So the namespace can't be overridden (and I don't think there's an option for watch current either)


In both cases, a RoleBinding not a ClusterRoleBinding is generated, which is what we're trying to find a property to do.

@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 10, 2023

Looks like JOSDK_WATCH_CURRENT does work today, but again cannot be overriden:

mvn clean package -Doperator -Dquarkus.container-image.build=true -Dquarkus.operator-sdk.controllers.keycloakcontroller.generate-with-watched-namespaces=JOSDK_WATCH_CURRENT
2023-07-10 20:30:10,028 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.deployment.Keycloak' for namespace(s): [openshift-operators]
2023-07-10 20:30:10,104 INFO [io.jav.ope.Operator] (main) Registered reconciler: 'keycloakrealmimportcontroller' for resource: 'class org.keycloak.operator.crds.v2alpha1.realmimport.KeycloakRealmImport' for namespace(s): [all namespaces]

@metacosm
Copy link
Member

I'm still struggling to understand what you're trying to achieve outside of the different options you've tried. Could you explain, from a user's perspective, the use case you're trying to fulfil? Then we can figure out if something needs to change in QOSDK or if it's a matter of better documentation. Thanks in advance.

@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 12, 2023

Was writing that up in the keycloak PR, so to save duplicating the thread: keycloak/keycloak#21231 (comment)

@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 13, 2023

Thanks for the call today.

I think we resolved that the behaviour of the QOSDK was buggy here:

  • The runtime operator level property for selecting namespaces should override the controller build time property for setting namespaces

Assuming that behaviour is changed, then this PR wouldn't be needed any more.

@Jamstah
Copy link
Contributor Author

Jamstah commented Jul 14, 2023

Closed by #654

@Jamstah Jamstah closed this Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants