Skip to content

Commit

Permalink
Fix #1107: fix findings
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolaferraro committed Jan 12, 2022
1 parent 567c2de commit 9d89922
Show file tree
Hide file tree
Showing 13 changed files with 157 additions and 173 deletions.
43 changes: 17 additions & 26 deletions addons/keda/keda.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import (
"github.com/apache/camel-k/pkg/util/source"
"github.com/apache/camel-k/pkg/util/uri"
"github.com/pkg/errors"
scase "github.com/stoewer/go-strcase"
autoscalingv1 "k8s.io/api/autoscaling/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -76,8 +76,6 @@ type kedaTrait struct {
trait.BaseTrait `property:",squash"`
// Enables automatic configuration of the trait. Allows the trait to infer KEDA triggers from the Kamelets.
Auto *bool `property:"auto" json:"auto,omitempty"`
// Convert metadata properties to camelCase (needed because Camel K trait properties use kebab-case from command line). Disabled by default.
CamelCaseConversion *bool `property:"camel-case-conversion" json:"camelCaseConversion,omitempty"`
// Set the spec->replicas field on the top level controller to an explicit value if missing, to allow KEDA to recognize it as a scalable resource.
HackControllerReplicas *bool `property:"hack-controller-replicas" json:"hackControllerReplicas,omitempty"`
// Interval (seconds) to check each trigger on.
Expand Down Expand Up @@ -170,11 +168,7 @@ func (t *kedaTrait) addScalingResources(e *trait.Environment) error {
for idx, trigger := range t.Triggers {
meta := make(map[string]string)
for k, v := range trigger.Metadata {
kk := k
if t.CamelCaseConversion != nil && *t.CamelCaseConversion {
kk = scase.LowerCamelCase(k)
}
meta[kk] = v
meta[k] = v
}
var authenticationRef *kedav1alpha1.ScaledObjectAuthRef
if len(trigger.authentication) > 0 && trigger.AuthenticationSecret != "" {
Expand Down Expand Up @@ -269,28 +263,25 @@ func (t *kedaTrait) addScalingResources(e *trait.Environment) error {

func (t *kedaTrait) hackControllerReplicas(e *trait.Environment) error {
ctrlRef := t.getTopControllerReference(e)
scale := autoscalingv1.Scale{
Spec: autoscalingv1.ScaleSpec{
Replicas: int32(1),
},
}
scalesClient, err := e.Client.ScalesClient()
if err != nil {
return err
}
if ctrlRef.Kind == camelv1alpha1.KameletBindingKind {
// Update the KameletBinding directly (do not add it to env resources, it's the integration parent)
key := ctrl.ObjectKey{
Namespace: e.Integration.Namespace,
Name: ctrlRef.Name,
}
klb := camelv1alpha1.KameletBinding{}
if err := e.Client.Get(e.Ctx, key, &klb); err != nil {
scale.ObjectMeta.Name = ctrlRef.Name
_, err = scalesClient.Scales(e.Integration.Namespace).Update(e.Ctx, camelv1alpha1.SchemeGroupVersion.WithResource("kameletbindings").GroupResource(), &scale, metav1.UpdateOptions{})
if err != nil {
return err
}
if klb.Spec.Replicas == nil {
one := int32(1)
klb.Spec.Replicas = &one
if err := e.Client.Update(e.Ctx, &klb); err != nil {
return err
}
}
} else if e.Integration.Spec.Replicas == nil {
one := int32(1)
e.Integration.Spec.Replicas = &one
// Update the Integration directly as the spec section is not merged by default
if err := e.Client.Update(e.Ctx, e.Integration); err != nil {
scale.ObjectMeta.Name = e.Integration.Name
_, err = scalesClient.Scales(e.Integration.Namespace).Update(e.Ctx, camelv1.SchemeGroupVersion.WithResource("integrations").GroupResource(), &scale, metav1.UpdateOptions{})
if err != nil {
return err
}
}
Expand Down
61 changes: 52 additions & 9 deletions addons/keda/keda_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
)

var (
Expand Down Expand Up @@ -348,14 +347,58 @@ func TestHackReplicas(t *testing.T) {
assert.NoError(t, err)
assert.True(t, res)
assert.NoError(t, keda.Apply(env))
it := camelv1.Integration{}
key := client.ObjectKey{
Namespace: "test",
Name: "my-it",
}
assert.NoError(t, env.Client.Get(env.Ctx, key, &it))
assert.NotNil(t, it.Spec.Replicas)
assert.Equal(t, int32(1), *it.Spec.Replicas)
scalesClient, err := env.Client.ScalesClient()
assert.NoError(t, err)
sc, err := scalesClient.Scales("test").Get(env.Ctx, camelv1.SchemeGroupVersion.WithResource("integrations").GroupResource(), "my-it", metav1.GetOptions{})
assert.NoError(t, err)
assert.Equal(t, int32(1), sc.Spec.Replicas)
}

func TestHackKLBReplicas(t *testing.T) {
keda, _ := NewKedaTrait().(*kedaTrait)
keda.Enabled = &testingTrue
keda.Auto = &testingFalse
keda.Triggers = append(keda.Triggers, kedaTrigger{
Type: "custom",
Metadata: map[string]string{
"a": "b",
},
})
keda.HackControllerReplicas = &testingTrue
env := createBasicTestEnvironment(
&camelv1alpha1.KameletBinding{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Name: "my-klb",
},
},
&camelv1.Integration{
ObjectMeta: metav1.ObjectMeta{
Namespace: "test",
Name: "my-it",
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: camelv1alpha1.SchemeGroupVersion.String(),
Kind: "KameletBinding",
Name: "my-klb",
},
},
},
Status: camelv1.IntegrationStatus{
Phase: camelv1.IntegrationPhaseInitialization,
},
},
)

res, err := keda.Configure(env)
assert.NoError(t, err)
assert.True(t, res)
assert.NoError(t, keda.Apply(env))
scalesClient, err := env.Client.ScalesClient()
assert.NoError(t, err)
sc, err := scalesClient.Scales("test").Get(env.Ctx, camelv1alpha1.SchemeGroupVersion.WithResource("kameletbindings").GroupResource(), "my-klb", metav1.GetOptions{})
assert.NoError(t, err)
assert.Equal(t, int32(1), sc.Spec.Replicas)
}

func getScaledObject(e *trait.Environment) *v1alpha1.ScaledObject {
Expand Down
4 changes: 0 additions & 4 deletions docs/modules/traits/pages/keda.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,6 @@ The following configuration options are available:
| bool
| Enables automatic configuration of the trait. Allows the trait to infer KEDA triggers from the Kamelets.

| keda.camel-case-conversion
| bool
| Convert metadata properties to camelCase (needed because Camel K trait properties use kebab-case from command line). Disabled by default.

| keda.hack-controller-replicas
| bool
| Set the spec->replicas field on the top level controller to an explicit value if missing, to allow KEDA to recognize it as a scalable resource.
Expand Down
11 changes: 1 addition & 10 deletions e2e/common/scale_binding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"k8s.io/client-go/dynamic"
"k8s.io/client-go/restmapper"
"k8s.io/client-go/scale"

. "github.com/apache/camel-k/e2e/support"
v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
"github.com/apache/camel-k/pkg/apis/camel/v1alpha1"
Expand Down Expand Up @@ -80,12 +76,7 @@ func TestKameletBindingScale(t *testing.T) {

t.Run("Scale kamelet binding with polymorphic client", func(t *testing.T) {
RegisterTestingT(t)
// Polymorphic scale client
groupResources, err := restmapper.GetAPIGroupResources(TestClient().Discovery())
Expect(err).To(BeNil())
mapper := restmapper.NewDiscoveryRESTMapper(groupResources)
resolver := scale.NewDiscoveryScaleKindResolver(TestClient().Discovery())
scaleClient, err := scale.NewForConfig(TestClient().GetConfig(), mapper, dynamic.LegacyAPIPathResolverFunc, resolver)
scaleClient, err := TestClient().ScalesClient()
Expect(err).To(BeNil())

// Patch the integration scale subresource
Expand Down
11 changes: 1 addition & 10 deletions e2e/common/scale_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,6 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"k8s.io/client-go/dynamic"
"k8s.io/client-go/restmapper"
"k8s.io/client-go/scale"

. "github.com/apache/camel-k/e2e/support"
v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
"github.com/apache/camel-k/pkg/client/camel/clientset/versioned"
Expand Down Expand Up @@ -67,12 +63,7 @@ func TestIntegrationScale(t *testing.T) {

t.Run("Scale integration with polymorphic client", func(t *testing.T) {
RegisterTestingT(t)
// Polymorphic scale client
groupResources, err := restmapper.GetAPIGroupResources(TestClient().Discovery())
Expect(err).To(BeNil())
mapper := restmapper.NewDiscoveryRESTMapper(groupResources)
resolver := scale.NewDiscoveryScaleKindResolver(TestClient().Discovery())
scaleClient, err := scale.NewForConfig(TestClient().GetConfig(), mapper, dynamic.LegacyAPIPathResolverFunc, resolver)
scaleClient, err := TestClient().ScalesClient()
Expect(err).To(BeNil())

// Patch the integration scale subresource
Expand Down
File renamed without changes.
2 changes: 2 additions & 0 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
user "github.com/mitchellh/go-homedir"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"k8s.io/client-go/scale"

"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
Expand Down Expand Up @@ -64,6 +65,7 @@ type Client interface {
GetConfig() *rest.Config
GetCurrentNamespace(kubeConfig string) (string, error)
ServerOrClientSideApplier() ServerOrClientSideApplier
ScalesClient() (scale.ScalesGetter, error)
}

// Injectable identifies objects that can receive a Client.
Expand Down
35 changes: 35 additions & 0 deletions pkg/client/scale.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
Licensed to the Apache Software Foundation (ASF) under one or more
contributor license agreements. See the NOTICE file distributed with
this work for additional information regarding copyright ownership.
The ASF licenses this file to You under the Apache License, Version 2.0
(the "License"); you may not use this file except in compliance with
the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package client

import (
"k8s.io/client-go/dynamic"
"k8s.io/client-go/restmapper"
"k8s.io/client-go/scale"
)

func (c *defaultClient) ScalesClient() (scale.ScalesGetter, error) {
// Polymorphic scale client
groupResources, err := restmapper.GetAPIGroupResources(c.Discovery())
if err != nil {
return nil, err
}
mapper := restmapper.NewDiscoveryRESTMapper(groupResources)
resolver := scale.NewDiscoveryScaleKindResolver(c.Discovery())
return scale.NewForConfig(c.GetConfig(), mapper, dynamic.LegacyAPIPathResolverFunc, resolver)
}
2 changes: 1 addition & 1 deletion pkg/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ import (
"github.com/apache/camel-k/pkg/util/watch"
)

var traitConfigRegexp = regexp.MustCompile(`^([a-z0-9-]+)((?:\[[0-9]+\]|\.[a-z0-9-]+)+)=(.*)$`)
var traitConfigRegexp = regexp.MustCompile(`^([a-z0-9-]+)((?:\.[a-z0-9-]+)(?:\[[0-9]+\]|\.[A-Za-z0-9-_]+)*)=(.*)$`)

func newCmdRun(rootCmdOptions *RootCmdOptions) (*cobra.Command, *runCmdOptions) {
options := runCmdOptions{
Expand Down
4 changes: 2 additions & 2 deletions pkg/resources/resources.go

Large diffs are not rendered by default.

Loading

0 comments on commit 9d89922

Please sign in to comment.