Skip to content

Commit 6ccf7ec

Browse files
Sharvashilya-lesikov
Sharvash
and
ilya-lesikov
authored
feat: add automatic deploy dependency detection for KEDA ScaledObjects (#631)
nelm now automatically deploys a KEDA `ScaledObject` after the workload referenced by its `spec.scaleTargetRef`, so the ordering no longer has to be declared by hand. Previously, when a `ScaledObject` and its target workload (e.g. a `Deployment`) belonged to the same release, nelm put both in the same deploy stage with no ordering between them. If the `ScaledObject` reached the API server before its target existed, the install broke in one of two ways: - with a `cpu`/`memory` trigger, KEDA's validating webhook resolves the target and denies the request (the error reported in #456): `admission webhook "vscaledobject.kb.io" denied the request: Deployment.apps "app" not found` - with any other trigger the object is admitted, but the KEDA controller cannot reconcile it and it stays `Ready=False` with `reason=ScaledObjectCheckFailed` The only way around it was a manual dependency annotation: ```yaml werf.io/deploy-dependency-app: state=present,kind=Deployment,name=app ``` `internalDeployDependencies` already derives implicit dependencies for built-in workloads, RBAC bindings and others. This adds a `keda.sh/ScaledObject` case that reads `spec.scaleTargetRef` and emits a `present` dependency on the referenced workload: - `name` - required; if it is missing the `ScaledObject` is left untouched - `kind` - defaults to `Deployment` - `apiVersion` - defaults to `apps/v1`, and the group is parsed from it The dependency is `present`, not `ready`: nelm waits only for the target to exist, which is all KEDA needs, instead of blocking on its readiness. `ScaledJob` is intentionally left out — it uses `jobTargetRef` and creates its own Jobs, so it has no such dependency. Verified against a live cluster running KEDA: with the change the `ScaledObject` is always created after its target and reconciles cleanly, and the #456 webhook denial reproduces without it. Closes #456 Signed-off-by: Alexey Gorovenko <sharvashinho@gmail.com> Co-authored-by: Ilya Lesikov <ilya@lesikov.com>
1 parent 9178d7f commit 6ccf7ec

2 files changed

Lines changed: 169 additions & 0 deletions

File tree

‎pkg/resource/dependency.go‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ func internalDeployDependencies(unstruct *unstructured.Unstructured, otherUnstru
8888
if deps, found := parseBindingSubjects(unstruct); found {
8989
dependencies = append(dependencies, deps...)
9090
}
91+
case schema.GroupKind{Kind: "ScaledObject", Group: "keda.sh"}:
92+
if dep, found := parseScaleTargetRef(unstruct); found {
93+
dependencies = append(dependencies, dep)
94+
}
9195
}
9296

9397
return dependencies
@@ -587,6 +591,40 @@ func parseRuntimeClassName(pod interface{}) (dep *InternalDependency, found bool
587591
return dep, true
588592
}
589593

594+
func parseScaleTargetRef(unstruct *unstructured.Unstructured) (dep *InternalDependency, found bool) {
595+
name, found := nestedStringNotEmpty(unstruct.Object, "spec", "scaleTargetRef", "name")
596+
if !found {
597+
return nil, false
598+
}
599+
600+
kind, found := nestedStringNotEmpty(unstruct.Object, "spec", "scaleTargetRef", "kind")
601+
if !found {
602+
kind = "Deployment"
603+
}
604+
605+
apiVersion, found := nestedStringNotEmpty(unstruct.Object, "spec", "scaleTargetRef", "apiVersion")
606+
if !found {
607+
apiVersion = "apps/v1"
608+
}
609+
610+
gv, err := schema.ParseGroupVersion(apiVersion)
611+
if err != nil {
612+
return nil, false
613+
}
614+
615+
dep = &InternalDependency{
616+
ResourceMatcher: &spec.ResourceMatcher{
617+
Names: []string{name},
618+
Namespaces: []string{unstruct.GetNamespace()},
619+
Groups: []string{gv.Group},
620+
Kinds: []string{kind},
621+
},
622+
ResourceState: common.ResourceStatePresent,
623+
}
624+
625+
return dep, true
626+
}
627+
590628
func parseSecretKeyRef(unstruct *unstructured.Unstructured, env interface{}) (dep *InternalDependency, found bool) {
591629
secretKeyRef, found := nestedMap(env, "valueFrom", "secretKeyRef")
592630
if !found {

‎pkg/resource/resource_test.go‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,137 @@ func (s *InstallableResourceSuite) TestNewInstallableResourceForDependencies() {
713713
}
714714
},
715715
},
716+
{
717+
expect: func(resSpec *spec.ResourceSpec) *resource.InstallableResource {
718+
res := defaultInstallableResource(resSpec)
719+
res.AutoInternalDependencies = []*resource.InternalDependency{
720+
{
721+
ResourceMatcher: &spec.ResourceMatcher{
722+
Names: []string{"test-statefulset"},
723+
Namespaces: []string{""},
724+
Groups: []string{"apps"},
725+
Kinds: []string{"StatefulSet"},
726+
},
727+
ResourceState: common.ResourceStatePresent,
728+
},
729+
}
730+
731+
return res
732+
},
733+
input: func() *spec.ResourceSpec {
734+
return spec.NewResourceSpec(&unstructured.Unstructured{
735+
Object: map[string]interface{}{
736+
"apiVersion": "keda.sh/v1alpha1",
737+
"kind": "ScaledObject",
738+
"metadata": map[string]interface{}{
739+
"name": "test-scaledobject",
740+
},
741+
"spec": map[string]interface{}{
742+
"scaleTargetRef": map[string]interface{}{
743+
"name": "test-statefulset",
744+
"kind": "StatefulSet",
745+
"apiVersion": "apps/v1",
746+
},
747+
},
748+
},
749+
}, s.releaseNamespace, spec.ResourceSpecOptions{})
750+
},
751+
name: `for ScaledObject resource with auto internal dependency on scale target`,
752+
},
753+
{
754+
expect: func(resSpec *spec.ResourceSpec) *resource.InstallableResource {
755+
res := defaultInstallableResource(resSpec)
756+
res.AutoInternalDependencies = []*resource.InternalDependency{
757+
{
758+
ResourceMatcher: &spec.ResourceMatcher{
759+
Names: []string{"test-deployment"},
760+
Namespaces: []string{""},
761+
Groups: []string{"apps"},
762+
Kinds: []string{"Deployment"},
763+
},
764+
ResourceState: common.ResourceStatePresent,
765+
},
766+
}
767+
768+
return res
769+
},
770+
input: func() *spec.ResourceSpec {
771+
return spec.NewResourceSpec(&unstructured.Unstructured{
772+
Object: map[string]interface{}{
773+
"apiVersion": "keda.sh/v1alpha1",
774+
"kind": "ScaledObject",
775+
"metadata": map[string]interface{}{
776+
"name": "test-scaledobject",
777+
},
778+
"spec": map[string]interface{}{
779+
"scaleTargetRef": map[string]interface{}{
780+
"name": "test-deployment",
781+
},
782+
},
783+
},
784+
}, s.releaseNamespace, spec.ResourceSpecOptions{})
785+
},
786+
name: `for ScaledObject resource with auto internal dependency on scale target with default kind and apiVersion`,
787+
},
788+
{
789+
expect: func(resSpec *spec.ResourceSpec) *resource.InstallableResource {
790+
return defaultInstallableResource(resSpec)
791+
},
792+
input: func() *spec.ResourceSpec {
793+
return spec.NewResourceSpec(&unstructured.Unstructured{
794+
Object: map[string]interface{}{
795+
"apiVersion": "keda.sh/v1alpha1",
796+
"kind": "ScaledObject",
797+
"metadata": map[string]interface{}{
798+
"name": "test-scaledobject",
799+
},
800+
"spec": map[string]interface{}{
801+
"scaleTargetRef": map[string]interface{}{
802+
"kind": "Deployment",
803+
},
804+
},
805+
},
806+
}, s.releaseNamespace, spec.ResourceSpecOptions{})
807+
},
808+
name: `for ScaledObject resource without scaleTargetRef name and therefore without auto internal dependency`,
809+
},
810+
{
811+
expect: func(resSpec *spec.ResourceSpec) *resource.InstallableResource {
812+
res := defaultInstallableResource(resSpec)
813+
res.AutoInternalDependencies = []*resource.InternalDependency{
814+
{
815+
ResourceMatcher: &spec.ResourceMatcher{
816+
Names: []string{"test-rollout"},
817+
Namespaces: []string{""},
818+
Groups: []string{"argoproj.io"},
819+
Kinds: []string{"Rollout"},
820+
},
821+
ResourceState: common.ResourceStatePresent,
822+
},
823+
}
824+
825+
return res
826+
},
827+
input: func() *spec.ResourceSpec {
828+
return spec.NewResourceSpec(&unstructured.Unstructured{
829+
Object: map[string]interface{}{
830+
"apiVersion": "keda.sh/v1alpha1",
831+
"kind": "ScaledObject",
832+
"metadata": map[string]interface{}{
833+
"name": "test-scaledobject",
834+
},
835+
"spec": map[string]interface{}{
836+
"scaleTargetRef": map[string]interface{}{
837+
"name": "test-rollout",
838+
"kind": "Rollout",
839+
"apiVersion": "argoproj.io/v1alpha1",
840+
},
841+
},
842+
},
843+
}, s.releaseNamespace, spec.ResourceSpecOptions{})
844+
},
845+
name: `for ScaledObject resource with auto internal dependency on custom resource scale target`,
846+
},
716847
}
717848

718849
for _, tc := range testCases {

0 commit comments

Comments
 (0)