update karpenter manifest management #744
Conversation
Signed-off-by: Mahmoud Gaballah <mahmoud.gaballah@zalando.de>
myaser
force-pushed
the
karpenter-update
branch
from
f9067dd
to
d992ad6
Compare
Signed-off-by: Mahmoud Gaballah <mahmoud.gaballah@zalando.de>
pkg/updatestrategy/aws_ec2.go
Outdated
| //if apierrors.IsNotFound(err) { | ||
| // // the node pool have been deleted. thus the | ||
| // return nil, nil | ||
| //} |
There was a problem hiding this comment.
nit: remove commented code
There was a problem hiding this comment.
🤔
actually, this may manage to mark old karpenter node-pools for decommission.
I will experiment with uncomment it and keep them to see if it makes a difference
|
|
||
| func NewLazyOf[T any](newfunc func() (T, error)) *LazyOf[T] { | ||
| return &LazyOf[T]{New: newfunc} | ||
| } |
There was a problem hiding this comment.
It's not completely clear to me why this lazy initialization is needed. Can't we just initialize when initializing the NodePool backend, or is it because the resources are not applied at that time?
The way to solve that would be to do the resolution whenever there is a need to lookup the CRD resources and not during the setup of the client. Since this is gonna be short lived I think we can keep it like this for now. But I would suggest to remove it long term to simplify the code.
There was a problem hiding this comment.
yes, it is because the CRDs are not there during the initialization of the backend.
I tried to delay the initialization of the resolver but it was a bit difficult to pass all the parameters across the functions all the way to where it is needed. I thought this was "simpler" 😄
But I would suggest to remove it long term to simplify the code.
yea, sure, we can do this
Signed-off-by: Mahmoud Gaballah <mahmoud.gaballah@zalando.de>
myaser
force-pushed
the
karpenter-update
branch
from
7cd6667
to
8c5e105
Compare
|
👍 |
1 similar comment
|
👍🏻 |
CLM had the CRDs for karpenter hardcoded.
these strings are used to query k8s api server searching for node pools to be cleaned up
the karpenter API is now upgraded from alpha to beta and thus the CRDs have been renamed and their structure have slightly changed
in this PR is adding support for both versions of karpenter (depends on a config_item)