Skip to content

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


// 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

.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
@korewaChino
Copy link
Member

korewaChino commented Jan 11, 2025

Could you please update this PR? Rebase it to the latest branch

@korewaChino
Copy link
Member

superceded by #200

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