chore: Version CRD #597

xeniape · 2025-02-12T08:01:39Z

Description

--- PASS: kuttl (750.67s)
    --- PASS: kuttl/harness (0.00s)
        --- PASS: kuttl/harness/resources_superset-latest-4.1.1_openshift-false (103.78s)
        --- PASS: kuttl/harness/cluster-operation_superset-latest-4.1.1_openshift-false (117.03s)
        --- PASS: kuttl/harness/logging_superset-4.0.2_openshift-false (185.71s)
        --- PASS: kuttl/harness/ldap_superset-4.1.1_ldap-authentication-server-verification-tls_openshift-false (137.80s)
        --- PASS: kuttl/harness/ldap_superset-4.1.1_ldap-authentication-no-tls_openshift-false (124.60s)
        --- PASS: kuttl/harness/ldap_superset-4.1.1_ldap-authentication-insecure-tls_openshift-false (100.91s)
        --- PASS: kuttl/harness/ldap_superset-4.0.2_ldap-authentication-server-verification-tls_openshift-false (103.86s)
        --- PASS: kuttl/harness/ldap_superset-4.0.2_ldap-authentication-no-tls_openshift-false (104.41s)
        --- PASS: kuttl/harness/ldap_superset-4.0.2_ldap-authentication-insecure-tls_openshift-false (101.61s)
        --- PASS: kuttl/harness/smoke_superset-4.1.1_openshift-false (96.96s)
        --- PASS: kuttl/harness/logging_superset-4.1.1_openshift-false (115.13s)
        --- PASS: kuttl/harness/oidc_superset-4.1.1_openshift-false (137.10s)
        --- PASS: kuttl/harness/oidc_superset-4.0.2_openshift-false (123.61s)
        --- PASS: kuttl/harness/smoke_superset-4.0.2_openshift-false (98.51s)
        --- PASS: kuttl/harness/druid-connection_superset-4.1.1_openshift-false (192.16s)
        --- PASS: kuttl/harness/druid-connection_superset-4.0.2_openshift-false (184.04s)
PASS

Definition of Done Checklist

Not all of these items are applicable to all PRs, the author should update this template to only leave the boxes in that are relevant
Please make sure all these things are done and tick the boxes

Author

Give feedback

Changes are OpenShift compatible
CRD changes approved
CRD documentation for all fields, following the style guide.
Helm chart can be installed and deployed operator works
Integration tests passed (for non trivial changes)
Changes need to be "offline" compatible
Options

Reviewer

Give feedback

Code contains useful comments
Code contains useful logging statements
(Integration-)Test cases added
Documentation added or updated. Follows the style guide.
Changelog updated
Cargo.toml only contains references to git tags (not specific commits or branches)
Options

Acceptance

Give feedback

Feature Tracker has been updated
Proper release label has been added
Roadmap has been updated
Options

NickLarsenNZ

Just some changes around the imports are required.

rust/operator-binary/src/config.rs

rust/operator-binary/src/main.rs

rust/operator-binary/src/operations/graceful_shutdown.rs

rust/operator-binary/src/operations/pdb.rs

rust/operator-binary/src/crd/mod.rs

rust/operator-binary/src/crd/affinity.rs

rust/operator-binary/src/crd/druidconnection.rs

rust/operator-binary/src/crd/mod.rs

Techassi · 2025-02-19T10:48:57Z

rust/operator-binary/src/crd/mod.rs

+    /// A Superset cluster stacklet. This resource is managed by the Stackable operator for Apache Superset.
+    /// Find more information on how to use it and the resources that the operator generates in the
+    /// [operator documentation](DOCS_BASE_URL_PLACEHOLDER/superset/).
+    #[derive(Clone, CustomResource, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]


question: Does this work (are the derives forwarded to the generated code)? Because we had trouble in the past, when these attributes appeared before the #[versioned] macro.

Ran cargo expand crd for both versions (setting the macro at the beginning and after another macro) and compared the whole code block between the SupersetClusterSpec docs and SupersetClusterConfig which should include everything generated for it I hope. And they are the same.
Or do you have an example where I would encounter those problems?

Or do you have an example where I would encounter those problems?

We encountered this issue once during development of the macro. That's why we advise to put the macro before any other (derive) macros. It is nice that it also seems to work the way you have done it.

Feel free to leave it as is or move it (which would also bring it in line with other PRs).

rust/operator-binary/src/crd/authentication.rs

rust/operator-binary/src/druid_connection_controller.rs

rust/operator-binary/src/main.rs

NickLarsenNZ

LGTM, just left a question about some un-versioned types.

NickLarsenNZ · 2025-02-25T08:25:30Z

rust/operator-binary/src/config.rs

    authentication::{
-        FlaskRolesSyncMoment, SupersetAuthenticationClassResolved,
-        SupersetClientAuthenticationDetailsResolved, DEFAULT_OIDC_PROVIDER,
+        self, SupersetAuthenticationClassResolved, SupersetClientAuthenticationDetailsResolved,


Just noting that these have not been versioned. Is that on purpose?

SupersetAuthenticationClassResolved

SupersetClientAuthenticationDetailsResolved

Techassi · 2025-02-25T09:22:53Z

rust/operator-binary/src/crd/druidconnection.rs

+        plural = "druidconnections",
+        status = "DruidConnectionStatus",
+        namespaced,


note: This works, but if we already version the status, we should explicitly use a version here.

Suggested change

plural = "druidconnections",

status = "DruidConnectionStatus",

namespaced,

plural = "druidconnections",

status = "v1alpha1::DruidConnectionStatus",

namespaced,

Waiting for this upstream change kube-rs/kube#1704 to update that.

Techassi · 2025-02-25T09:37:16Z

rust/operator-binary/src/main.rs

@@ -1,6 +1,7 @@
 use std::sync::Arc;

 use clap::{crate_description, crate_version, Parser};
+use crd::druidconnection::DruidConnection;


note: This should instead be moved into the crate:: imports below.

sbernauer · 2025-03-03T08:06:58Z

Currently waiting for kube-rs/kube#1704

xeniape added 4 commits February 11, 2025 15:26

chore: migrate crd crate to operator-binary module

ae9f967

chore: add stackable-versioned dependency

486df06

chore: separate impls from crd data structures

81bd43a

chore: version the crd

6e604b5

xeniape self-assigned this Feb 12, 2025

xeniape mentioned this pull request Feb 12, 2025

tracking: CRD versioning stackabletech/issues#642

Open

xeniape marked this pull request as ready for review February 12, 2025 09:01

NickLarsenNZ self-requested a review February 12, 2025 14:12

NickLarsenNZ requested changes Feb 12, 2025

View reviewed changes

bump stackable-versioned, refactor version imports and references

b3b5ec1

xeniape requested a review from NickLarsenNZ February 17, 2025 12:44

Techassi requested changes Feb 19, 2025

View reviewed changes

xeniape added 2 commits February 24, 2025 12:57

adjust version references, remove redundant code

21082f3

Merge branch 'main' into chore/add-crd-versioning

5683ec9

xeniape requested a review from Techassi February 24, 2025 12:10

NickLarsenNZ approved these changes Feb 25, 2025

View reviewed changes

Techassi requested changes Feb 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Version CRD #597

chore: Version CRD #597

xeniape commented Feb 12, 2025 •

edited

Loading

Author

Reviewer

Acceptance

NickLarsenNZ left a comment

Techassi Feb 19, 2025

xeniape Feb 24, 2025

Techassi Feb 25, 2025

NickLarsenNZ left a comment

NickLarsenNZ Feb 25, 2025 •

edited

Loading

Techassi Feb 25, 2025

xeniape Feb 28, 2025

Techassi Feb 25, 2025

sbernauer commented Mar 3, 2025

chore: Version CRD #597

Are you sure you want to change the base?

chore: Version CRD #597

Conversation

xeniape commented Feb 12, 2025 • edited Loading

Description

Definition of Done Checklist

Author

Reviewer

Acceptance

NickLarsenNZ left a comment

Choose a reason for hiding this comment

Techassi Feb 19, 2025

Choose a reason for hiding this comment

xeniape Feb 24, 2025

Choose a reason for hiding this comment

Techassi Feb 25, 2025

Choose a reason for hiding this comment

NickLarsenNZ left a comment

Choose a reason for hiding this comment

NickLarsenNZ Feb 25, 2025 • edited Loading

Choose a reason for hiding this comment

Techassi Feb 25, 2025

Choose a reason for hiding this comment

xeniape Feb 28, 2025

Choose a reason for hiding this comment

Techassi Feb 25, 2025

Choose a reason for hiding this comment

sbernauer commented Mar 3, 2025

xeniape commented Feb 12, 2025 •

edited

Loading

NickLarsenNZ Feb 25, 2025 •

edited

Loading