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

Added ability to limit the LoadBalancers picked up by chisel-operator #153

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

bw210343
Copy link

@bw210343 bw210343 commented Jan 5, 2025

Configured this locally and noticed @xyhhx has created a request for the capability - #150

Uses an environment variable REQUIRE_OPERATOR_CLASS to determine whether the LoadBalancers picked up by chisel-operator should be limited.

Includes configuration updates for the helm chart

@xyhhx
Copy link

xyhhx commented Jan 6, 2025

did i just get nerd sniped

@korewaChino
Copy link
Member

did i just get nerd sniped

yea

@@ -301,6 +303,13 @@ async fn exit_node_for_service(
async fn reconcile_svcs(obj: Arc<Service>, ctx: Arc<Context>) -> Result<Action, ReconcileError> {
// Return if service is not LoadBalancer or if the loadBalancerClass is not blank or set to $OPERATOR_CLASS

// Check if the REQUIRE_OPERATOR_CLASS environment variable is set
let limit_load_balancer_class;
match env::var("REQUIRE_OPERATOR_CLASS") {
Copy link
Member

Choose a reason for hiding this comment

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

REQUIRE_OPERATOR_CLASS's envar key should be a const in case the name needs to be changed, so one can easily change them in case it does need to be changed

let limit_load_balancer_class;
match env::var("REQUIRE_OPERATOR_CLASS") {
Ok(v) => limit_load_balancer_class = v,
Err(_e) => limit_load_balancer_class = "false".to_string(),
Copy link
Member

Choose a reason for hiding this comment

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

This could be a bit more elegant by using unwrap_or_else instead of a named block

@@ -311,7 +320,7 @@ async fn reconcile_svcs(obj: Arc<Service>, ctx: Arc<Context>) -> Result<Action,
.spec
.as_ref()
.filter(|spec| {
spec.load_balancer_class.is_none()
(spec.load_balancer_class.is_none() && ( limit_load_balancer_class == "false"))
Copy link
Member

Choose a reason for hiding this comment

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

You could also cast the boolean to a string using FromStr

@korewaChino
Copy link
Member

Oops, I forgot to submit my review :P

@korewaChino korewaChino linked an issue Jan 8, 2025 that may be closed by this pull request
@korewaChino korewaChino added the enhancement New feature or request label Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

exclude a loadbalancer from being picked up by chisel-operator?
4 participants