Repository navigation
feat: Kafka agent - #1028
feat: Kafka agent#1028labrenbe wants to merge 17 commits into
Conversation
| .resources( | ||
| ResourceRequirementsBuilder::new() | ||
| .with_cpu_request("100m") | ||
| .with_cpu_limit("500m") | ||
| .with_memory_request("128Mi") | ||
| .with_memory_limit("128Mi") | ||
| .build(), | ||
| ); |
There was a problem hiding this comment.
I think we should not hardcode the resources but offer them (in the CRD, not just overrides) and use defaults?
|
|
||
| [workspace.dependencies] | ||
| stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.118.0", features = ["crds", "webhook"] } | ||
| stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", branch = "feat/platform-access", features = ["crds", "webhook"] } |
There was a problem hiding this comment.
TODO: revert to a tag when ready.
| pub struct KafkaPlatformAccess { | ||
| /// Whether the operator deploys the agent. False by default. | ||
| #[serde(default)] | ||
| pub enabled: bool, |
There was a problem hiding this comment.
Why do we need this field if the whole struct is an Option? Is it just so that KafkaPlatformAccessAuthentication can be left in the CR (and turned on or off)? If so, that is a different pattern used by e.g. clusterConfig.authentication (which is Vec and which is either present or not) and we would need a good reason to use it.
| .with_labels(recommended_labels_for_agent_resources(cluster)) | ||
| .build(), | ||
| ) | ||
| .image_pull_secrets_from_product_image(&cluster.image) |
There was a problem hiding this comment.
Assuming that the agent image can use the same credentials as the product image is fine as long as they come from the same registry. If they don't, there should be an optional field platformAccess.image.pullSecrets that allows the user to specify these independently; when unset, the agent falls back to spec.image.pullSecrets as it currently does.
| ) | ||
| .owns( | ||
| watch_namespace.get_api::<DeserializeGuard<Deployment>>(&client), | ||
| // The agent is a the only Deployment |
There was a problem hiding this comment.
Typo or incomplete comment? Something like: "The agent Deployment is the only one the operator owns"?
| .image(&agent_config.image) | ||
| .command(vec!["stackable-kafka-agent".to_owned()]) | ||
| .args(vec!["run".to_owned()]) | ||
| .add_env_var_from_source( |
There was a problem hiding this comment.
This reverts to the pre-smooth-op changes: the pattern that should now be used is to have the env-var names defined as constants:
constant!(KUBERNETES_NODE_NAME: EnvVarName = "KUBERNETES_NODE_NAME");
constant!(KUBERNETES_CLUSTER_DOMAIN: EnvVarName = "KUBERNETES_CLUSTER_DOMAIN");
and to use these in e.g. a helper function that returns an EnvVarSet (the .expect is safe because the name is a compile-time constant) e.g.
fn agent_env_vars(cluster: &ValidatedCluster) -> EnvVarSet {
let node_name_env_var = EnvVar {
name: KUBERNETES_NODE_NAME.to_string(),
value: None,
value_from: Some(EnvVarSource {
field_ref: Some(ObjectFieldSelector {
field_path: "spec.nodeName".to_owned(),
..ObjectFieldSelector::default()
}),
..EnvVarSource::default()
}),
};
EnvVarSet::new()
// Saves the agent from looking the domain up via the kubelet, which needs extra RBAC.
.with_value(
&KUBERNETES_CLUSTER_DOMAIN,
cluster.cluster_domain.to_string(),
)
.with_env_var(node_name_env_var)
.expect("the env var name is a valid EnvVarName")
}
and then add this to the ContainerBuilder inline e.g.
cb_agent
...
.add_env_vars(agent_env_vars(cluster))
| } | ||
| } | ||
|
|
||
| pub fn build_agent_service_account(cluster: &ValidatedCluster) -> ServiceAccount { |
There was a problem hiding this comment.
rbac.rs delegates to op-rs https://github.com/stackabletech/kafka-operator/blob/feat/kafka-agent/rust/operator-binary/src/controller/build/resource/rbac.rs#L11-L27 but here they are built manually. If we add an agent-aware AgentResourceNames to op-rs, and let the v2::rbac builders take either name type (e.g. via a small trait over service_account_name / role_binding_name / cluster_role_name), we could do the same thing here. Then we could do this:
controller.rs:
pub fn agent_resource_names(&self) -> AgentResourceNames {
AgentResourceNames {
cluster_name: self.name.clone(),
product_name: PRODUCT_NAME.clone(),
}
}
agent.rs:
pub fn build_agent_service_account(cluster: &ValidatedCluster) -> ServiceAccount {
rbac::build_service_account(
cluster,
&cluster.agent_resource_names(),
recommended_labels_for_agent_resources(cluster),
)
}
pub fn build_agent_role_binding(cluster: &ValidatedCluster) -> RoleBinding {
rbac::build_role_binding(
cluster,
&cluster.agent_resource_names(),
recommended_labels_for_agent_resources(cluster),
)
}
9b6f2a7 to
146602d
Compare
Description
Part of stackabletech/issues#888.
Depends on decisions
Depends on PRs
Deploys an agent alongside Kafka clusters which is an independent controller that reconciles and manages internal resources like
KafkaTopic.This PR only adds an empty agent shell that doesn't do anything yet.
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecationlabel & add to the deprecation scheduletype/experimentallabel & add to the experimental features tracker