Skip to content

WIP: Initial Operator Implementation#13

Merged
fhennig merged 61 commits intomainfrom
start
Nov 12, 2021
Merged

WIP: Initial Operator Implementation#13
fhennig merged 61 commits intomainfrom
start

Conversation

@fhennig
Copy link
Copy Markdown
Contributor

@fhennig fhennig commented Nov 4, 2021

No description provided.

@fhennig fhennig requested a review from a team November 4, 2021 08:37
@fhennig fhennig marked this pull request as ready for review November 4, 2021 09:10
@fhennig
Copy link
Copy Markdown
Contributor Author

fhennig commented Nov 5, 2021

I just realized that to try it out yourself, you need to install the package manually since it isn't in the nexus - respectively the nexus version doesn't contain the custom startup script.

@fhennig
Copy link
Copy Markdown
Contributor Author

fhennig commented Nov 5, 2021

Okay, it was decided that the operator will be reviewed with the changes made to use the docker images already, I'll finish that up and add it to this pull request.

@fhennig fhennig marked this pull request as draft November 5, 2021 08:44
@fhennig fhennig requested a review from razvan November 5, 2021 08:47
@fhennig
Copy link
Copy Markdown
Contributor Author

fhennig commented Nov 10, 2021

looks good to me at this point

@fhennig fhennig marked this pull request as ready for review November 12, 2021 10:23
Copy link
Copy Markdown
Member

@razvan razvan left a comment

Choose a reason for hiding this comment

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

good work.

@fhennig fhennig merged commit e6e5ba7 into main Nov 12, 2021
@fhennig fhennig deleted the start branch November 12, 2021 10:41
Copy link
Copy Markdown
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.

Mostly left over commented code.

Comment on lines +29 to +33
assets = [
["../../target/release/stackable-druid-operator", "opt/stackable/druid-operator/", "755"],
["../../deploy/crd/druidcluster.crd.yaml", "etc/stackable/druid-operator/crd/", "644"],
["../../deploy/config-spec/properties.yaml", "etc/stackable/druid-operator/config-spec/", "644"],
]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not required anymore, just for completeness:

    ["../../deploy/crd/restart.crd.yaml", "etc/stackable/druid-operator/crd/", "644"],
    ["../../deploy/crd/start.crd.yaml", "etc/stackable/druid-operator/crd/", "644"],
    ["../../deploy/crd/stop.crd.yaml", "etc/stackable/druid-operator/crd/", "644"],

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.

The whole package.metadata.deb can be removed right? We don't build debian packages anymore I believe

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.

Ahhh now I see what you meant

Comment on lines +325 to +334
let runtime_properties = get_runtime_properties(role, &transformed_config);
cm_conf_data.insert(RUNTIME_PROPS.to_string(), runtime_properties);
}
PropertyNameKind::File(file_name) if file_name == JVM_CONFIG => {
let jvm_config = get_jvm_config(role);
cm_conf_data.insert(JVM_CONFIG.to_string(), jvm_config);
}
PropertyNameKind::File(file_name) if file_name == LOG4J2_CONFIG => {
let log_config = get_log4j_config(role);
cm_conf_data.insert(LOG4J2_CONFIG.to_string(), log_config);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The way this is build we never actually use the config overrides.
Please check e.g. https://github.com/stackabletech/hdfs-operator/blob/aab7c8b190af3700656c4996ad89c2cbba63b7a8/rust/operator/src/lib.rs#L293

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 see, I'm working on it

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.

Okay, I think for the runtime properties the transformed_config is already used, so that should work right? And the other two are not templated, so it's not possible to implement there.

@fhennig fhennig restored the start branch November 12, 2021 12:32
stackable-bot pushed a commit that referenced this pull request Nov 26, 2021
…ackabletech/operator-templating repo.

Original commit message:
Patch chart, docker and oprerator versions in pull-requests (#13)

* Update trino clusterrole rules.

* Patch versions on pull requests.

* Moved Python script to subfolder

* Fix retired files path.

* Fix regorule ClusterRole and uncomment all repos.

* Comment out restart-controler from repo list.
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.

3 participants