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

Discovery #102

Merged
merged 16 commits into from
Jan 18, 2022
Merged

Discovery #102

merged 16 commits into from
Jan 18, 2022

Conversation

fhennig
Copy link
Contributor

@fhennig fhennig commented Jan 11, 2022

Description

  • Added discovery to druid: The SQL connect string is written to a config map
  • Fixed the role services - they were referencing a port by name that doesn't have that name anymore

Review Checklist

  • Code contains useful comments
  • (Integration-)Test cases added (or not applicable)
  • Documentation added (or not applicable)
  • Changelog updated (or not applicable)
  • Cargo.toml only contains references to git tags (not specific commits or branches)

Once the review is done, comment bors r+ (or bors merge) to merge. Further information

@fhennig fhennig marked this pull request as ready for review January 12, 2022 10:30
@fhennig fhennig marked this pull request as draft January 12, 2022 10:46
@fhennig fhennig marked this pull request as ready for review January 12, 2022 11:10
@fhennig fhennig marked this pull request as draft January 12, 2022 11:10
@fhennig fhennig marked this pull request as ready for review January 12, 2022 12:38
Copy link
Member

@maltesander maltesander left a comment

Choose a reason for hiding this comment

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

LGTM.

@fhennig
Copy link
Contributor Author

fhennig commented Jan 13, 2022

bors merge

bors bot added a commit that referenced this pull request Jan 13, 2022
102: Discovery r=fhennig a=fhennig

## Description

- Added discovery to druid: The SQL connect string is written to a config map
- Fixed the role services - they were referencing a port by name that doesn't have that name anymore



Co-authored-by: Felix Hennig <[email protected]>
Co-authored-by: Felix Hennig <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 13, 2022

Build failed:

@fhennig
Copy link
Contributor Author

fhennig commented Jan 14, 2022

bors merge

bors bot added a commit that referenced this pull request Jan 14, 2022
102: Discovery r=fhennig a=fhennig

## Description

- Added discovery to druid: The SQL connect string is written to a config map
- Fixed the role services - they were referencing a port by name that doesn't have that name anymore



Co-authored-by: Felix Hennig <[email protected]>
Co-authored-by: Felix Hennig <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 14, 2022

Build failed:

@fhennig
Copy link
Contributor Author

fhennig commented Jan 18, 2022

bors merge

@bors
Copy link
Contributor

bors bot commented Jan 18, 2022

@bors bors bot merged commit b776439 into main Jan 18, 2022
@bors bors bot deleted the discovery branch January 18, 2022 14:35
bors bot pushed a commit to stackabletech/superset-operator that referenced this pull request Feb 1, 2022
## Description

This PR refactors the init process and adds a new custom resource for druid connections.

The Init resource is now called SupersetDB and also tracks a status. it is first `Provisioned`. If possible (secret also deployed) a kubernetes `Job` is started and the status of the CR becomes `Initializing`. The job is watched, and depending on whether it successfully completes or not, the SupersetDB also becomes `Ready` or `Failed` respectively.

This change allows us to also depend on the DB being ready (or not). This is necessary for writing the druid connection information into the DB.

A new CRD is introduced: `DruidConnection`. It holds information about the superset DB it should write to, and the Druid cluster that should be connected. A new controller watches the discovery config map as well as the superset DB resources. Once all the dependencies are there (and `Ready`) a job is started, and the status is updated, just like it is done for the `SupersetDB`.

This is in line with the declarative philosophy of k8s; the spec reflects the desired state and the status tracks the actual underlying cluster. Also, with multiple controllers watching object states, control flow is delegated a bit to the API server. Before we were watching the job state manually, now we react to status changes which are given to us by the API server. The controllers only react to object changes.

This PR also relies on the Druid Discovery PR stackabletech/druid-operator#102

What wasn't done: If the DruidConnection resource is deleted, the actual connection is not deleted. The issue doesn't say that this is necessary, and there is no obvious, documented way to delete data sources from the commandline, we'd have to go into the database and delete stuff there. On the controller side we can use a [finalizer](https://github.com/kube-rs/kube-rs/blob/d90ee9706923ceddf94c3d50928bc390543af2fc/examples/secret_syncer.rs#L92-L102).



Co-authored-by: Felix Hennig <[email protected]>
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