This is an automated email from the ASF dual-hosted git repository.
astefanutti pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel-k.git
The following commit(s) were added to refs/heads/main by this push:
new cd0a143 fix: Filter influencing traits to lookup matching kits
cd0a143 is described below
commit cd0a143bd4338d7b183f877f6a23221f09dbba7e
Author: Antonin Stefanutti <anto...@stefanutti.fr>
AuthorDate: Mon Jun 28 14:39:18 2021 +0200
fix: Filter influencing traits to lookup matching kits
---
pkg/controller/integration/build_kit.go | 178 +++++++++++++++++-
.../{util_test.go => build_kit_test.go} | 98 ++++------
.../integration/integration_controller.go | 2 +-
pkg/controller/integration/util.go | 200 ---------------------
pkg/util/test/trait.go | 2 -
5 files changed, 211 insertions(+), 269 deletions(-)
diff --git a/pkg/controller/integration/build_kit.go
b/pkg/controller/integration/build_kit.go
index be7cd63..30a629f 100644
--- a/pkg/controller/integration/build_kit.go
+++ b/pkg/controller/integration/build_kit.go
@@ -19,20 +19,26 @@ package integration
import (
"context"
+ "encoding/json"
"fmt"
+ "github.com/pkg/errors"
"github.com/rs/xid"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
+ "k8s.io/apimachinery/pkg/selection"
+
+ ctrl "sigs.k8s.io/controller-runtime/pkg/client"
v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
"github.com/apache/camel-k/pkg/platform"
"github.com/apache/camel-k/pkg/trait"
"github.com/apache/camel-k/pkg/util"
+ "github.com/apache/camel-k/pkg/util/controller"
+ "github.com/apache/camel-k/pkg/util/kubernetes"
)
-// NewBuildKitAction create an action that handles integration kit build
-func NewBuildKitAction() Action {
+func newBuildKitAction() Action {
return &buildKitAction{}
}
@@ -50,7 +56,7 @@ func (action *buildKitAction) CanHandle(integration
*v1.Integration) bool {
}
func (action *buildKitAction) Handle(ctx context.Context, integration
*v1.Integration) (*v1.Integration, error) {
- kit, err := LookupKitForIntegration(ctx, action.client, integration)
+ kit, err := action.lookupKitForIntegration(ctx, action.client,
integration)
if err != nil {
// TODO: we may need to add a wait strategy, i.e give up after
some time
return nil, err
@@ -159,3 +165,169 @@ func (action *buildKitAction) filterKitTraits(ctx
context.Context, in map[string
}
return out
}
+
+func (action *buildKitAction) lookupKitForIntegration(ctx context.Context, c
ctrl.Reader, integration *v1.Integration) (*v1.IntegrationKit, error) {
+ if integration.Status.IntegrationKit != nil {
+ kit, err := kubernetes.GetIntegrationKit(ctx, c,
integration.Status.IntegrationKit.Name,
integration.Status.IntegrationKit.Namespace)
+ if err != nil {
+ return nil, errors.Wrapf(err, "unable to find
integration kit %s/%s, %s", integration.Status.IntegrationKit.Namespace,
integration.Status.IntegrationKit.Name, err)
+ }
+
+ return kit, nil
+ }
+
+ pl, err := platform.GetCurrent(ctx, c, integration.Namespace)
+ if err != nil && !k8serrors.IsNotFound(err) {
+ return nil, err
+ }
+
+ options := []ctrl.ListOption{
+ ctrl.InNamespace(integration.GetIntegrationKitNamespace(pl)),
+ ctrl.MatchingLabels{
+ "camel.apache.org/runtime.version":
integration.Status.RuntimeVersion,
+ "camel.apache.org/runtime.provider":
string(integration.Status.RuntimeProvider),
+ },
+ controller.NewLabelSelector("camel.apache.org/kit.type",
selection.In, []string{
+ v1.IntegrationKitTypePlatform,
+ v1.IntegrationKitTypeExternal,
+ }),
+ }
+
+ kits := v1.NewIntegrationKitList()
+ if err := c.List(ctx, &kits, options...); err != nil {
+ return nil, err
+ }
+
+ for _, kit := range kits.Items {
+ kit := kit // pin
+
+ if kit.Status.Phase == v1.IntegrationKitPhaseError {
+ continue
+ }
+
+ /*
+ TODO: moved to label selector
+ if kit.Status.RuntimeVersion !=
integration.Status.RuntimeVersion {
+ continue
+ }
+ if kit.Status.RuntimeProvider !=
integration.Status.RuntimeProvider {
+ continue
+ }
+ */
+
+ if kit.Status.Version != integration.Status.Version {
+ continue
+ }
+
+ ideps := len(integration.Status.Dependencies)
+ cdeps := len(kit.Spec.Dependencies)
+
+ if ideps != cdeps {
+ continue
+ }
+
+ // When a platform kit is created it inherits the traits from
the integrations and as
+ // some traits may influence the build thus the artifacts
present on the container image,
+ // we need to take traits into account when looking up for
compatible kits.
+ //
+ // It could also happen that an integration is updated and a
trait is modified, if we do
+ // not include traits in the lookup, we may use a kit that does
not have all the
+ // characteristics required by the integration.
+ //
+ // A kit can be used only if it contains a subset of the traits
and related configurations
+ // declared on integration.
+ match, err := action.hasMatchingTraits(ctx, &kit, integration)
+ if err != nil {
+ return nil, err
+ }
+ if !match {
+ continue
+ }
+ if util.StringSliceContains(kit.Spec.Dependencies,
integration.Status.Dependencies) {
+ return &kit, nil
+ }
+ }
+
+ return nil, nil
+}
+
+// hasMatchingTraits compares traits defined on kit against those defined on
integration
+func (action *buildKitAction) hasMatchingTraits(ctx context.Context, kit
*v1.IntegrationKit, integration *v1.Integration) (bool, error) {
+ traits := action.filterKitTraits(ctx, integration.Spec.Traits)
+
+ // The kit has no trait, but the integration need some
+ if len(kit.Spec.Traits) == 0 && len(traits) > 0 {
+ return false, nil
+ }
+ for name, kitTrait := range kit.Spec.Traits {
+ itTrait, ok := traits[name]
+ if !ok {
+ // skip it because trait configured on kit is not
defined on integration
+ return false, nil
+ }
+ data, err := json.Marshal(itTrait.Configuration)
+ if err != nil {
+ return false, err
+ }
+ itConf := make(map[string]interface{})
+ err = json.Unmarshal(data, &itConf)
+ if err != nil {
+ return false, err
+ }
+ data, err = json.Marshal(kitTrait.Configuration)
+ if err != nil {
+ return false, err
+ }
+ kitConf := make(map[string]interface{})
+ err = json.Unmarshal(data, &kitConf)
+ if err != nil {
+ return false, err
+ }
+ for ck, cv := range kitConf {
+ iv, ok := itConf[ck]
+ if !ok {
+ // skip it because trait configured on kit has
a value that is not defined
+ // in integration trait
+ return false, nil
+ }
+ if !equal(iv, cv) {
+ // skip it because trait configured on kit has
a value that differs from
+ // the one configured on integration
+ return false, nil
+ }
+ }
+ }
+
+ return true, nil
+}
+
+// We need to try to perform a slice equality in order to prevent a runtime
panic
+func equal(a, b interface{}) bool {
+ aSlice, aOk := a.([]interface{})
+ bSlice, bOk := b.([]interface{})
+
+ if aOk && bOk {
+ // Both are slices
+ return sliceEqual(aSlice, bSlice)
+ }
+
+ if aOk || bOk {
+ // One of the 2 is a slice
+ return false
+ }
+
+ // None is a slice
+ return a == b
+}
+
+func sliceEqual(a, b []interface{}) bool {
+ if len(a) != len(b) {
+ return false
+ }
+ for i, v := range a {
+ if v != b[i] {
+ return false
+ }
+ }
+ return true
+}
diff --git a/pkg/controller/integration/util_test.go
b/pkg/controller/integration/build_kit_test.go
similarity index 77%
rename from pkg/controller/integration/util_test.go
rename to pkg/controller/integration/build_kit_test.go
index 0ec0846..6d1489f 100644
--- a/pkg/controller/integration/util_test.go
+++ b/pkg/controller/integration/build_kit_test.go
@@ -21,12 +21,13 @@ import (
"context"
"testing"
+ "github.com/stretchr/testify/assert"
+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
+ "github.com/apache/camel-k/pkg/util/log"
"github.com/apache/camel-k/pkg/util/test"
-
- "github.com/stretchr/testify/assert"
)
func TestLookupKitForIntegration_DiscardKitsInError(t *testing.T) {
@@ -79,7 +80,11 @@ func TestLookupKitForIntegration_DiscardKitsInError(t
*testing.T) {
assert.Nil(t, err)
- i, err := LookupKitForIntegration(context.TODO(), c, &v1.Integration{
+ a := buildKitAction{}
+ a.InjectLogger(log.Log)
+ a.InjectClient(c)
+
+ i, err := a.lookupKitForIntegration(context.TODO(), c, &v1.Integration{
TypeMeta: metav1.TypeMeta{
APIVersion: v1.SchemeGroupVersion.String(),
Kind: v1.IntegrationKind,
@@ -104,8 +109,7 @@ func TestLookupKitForIntegration_DiscardKitsInError(t
*testing.T) {
func TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t
*testing.T) {
c, err := test.NewFakeClient(
//
- // Should be discarded because it contains both of the required
traits but one
- // contains a different configuration value
+ // Should be discarded because it does not contain the required
traits
//
&v1.IntegrationKit{
TypeMeta: metav1.TypeMeta{
@@ -124,14 +128,6 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
"camel-core",
"camel-irc",
},
- Traits: map[string]v1.TraitSpec{
- "knative": test.TraitSpecFromMap(t,
map[string]interface{}{
- "enabled": "true",
- }),
- "knative-service":
test.TraitSpecFromMap(t, map[string]interface{}{
- "enabled": "false",
- }),
- },
},
Status: v1.IntegrationKitStatus{
Phase: v1.IntegrationKitPhaseReady,
@@ -159,7 +155,7 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
"camel-irc",
},
Traits: map[string]v1.TraitSpec{
- "knative": test.TraitSpecFromMap(t,
map[string]interface{}{
+ "builder": test.TraitSpecFromMap(t,
map[string]interface{}{
"enabled": "false",
}),
},
@@ -169,43 +165,6 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
},
},
//
- // Should be discarded because it contains both of the required
traits but
- // also an additional one
- //
- &v1.IntegrationKit{
- TypeMeta: metav1.TypeMeta{
- APIVersion: v1.SchemeGroupVersion.String(),
- Kind: v1.IntegrationKitKind,
- },
- ObjectMeta: metav1.ObjectMeta{
- Namespace: "ns",
- Name: "my-kit-3",
- Labels: map[string]string{
- "camel.apache.org/kit.type":
v1.IntegrationKitTypePlatform,
- },
- },
- Spec: v1.IntegrationKitSpec{
- Dependencies: []string{
- "camel-core",
- "camel-irc",
- },
- Traits: map[string]v1.TraitSpec{
- "knative": test.TraitSpecFromMap(t,
map[string]interface{}{
- "enabled": "true",
- }),
- "knative-service":
test.TraitSpecFromMap(t, map[string]interface{}{
- "enabled": "true",
- }),
- "gc": test.TraitSpecFromMap(t,
map[string]interface{}{
- "enabled": "true",
- }),
- },
- },
- Status: v1.IntegrationKitStatus{
- Phase: v1.IntegrationKitPhaseReady,
- },
- },
- //
// Should NOT be discarded because it contains a subset of the
required traits and
// same configuration values
//
@@ -216,7 +175,7 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
},
ObjectMeta: metav1.ObjectMeta{
Namespace: "ns",
- Name: "my-kit-4",
+ Name: "my-kit-3",
Labels: map[string]string{
"camel.apache.org/kit.type":
v1.IntegrationKitTypePlatform,
},
@@ -227,8 +186,11 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
"camel-irc",
},
Traits: map[string]v1.TraitSpec{
- "knative": test.TraitSpecFromMap(t,
map[string]interface{}{
+ "builder": test.TraitSpecFromMap(t,
map[string]interface{}{
"enabled": "true",
+ "properties": []string{
+
"build-key1=build-value1",
+ },
}),
},
},
@@ -240,7 +202,11 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
assert.Nil(t, err)
- i, err := LookupKitForIntegration(context.TODO(), c, &v1.Integration{
+ a := buildKitAction{}
+ a.InjectLogger(log.Log)
+ a.InjectClient(c)
+
+ i, err := a.lookupKitForIntegration(context.TODO(), c, &v1.Integration{
TypeMeta: metav1.TypeMeta{
APIVersion: v1.SchemeGroupVersion.String(),
Kind: v1.IntegrationKind,
@@ -251,11 +217,11 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
},
Spec: v1.IntegrationSpec{
Traits: map[string]v1.TraitSpec{
- "knative": test.TraitSpecFromMap(t,
map[string]interface{}{
- "enabled": "true",
- }),
- "knative-service": test.TraitSpecFromMap(t,
map[string]interface{}{
+ "builder": test.TraitSpecFromMap(t,
map[string]interface{}{
"enabled": "true",
+ "properties": []string{
+ "build-key1=build-value1",
+ },
}),
},
},
@@ -269,7 +235,7 @@ func
TestLookupKitForIntegration_DiscardKitsWithIncompatibleTraits(t *testing.T)
assert.Nil(t, err)
assert.NotNil(t, i)
- assert.Equal(t, "my-kit-4", i.Name)
+ assert.Equal(t, "my-kit-3", i.Name)
}
func TestHasMatchingTraits_KitNoTraitShouldNotBePicked(t *testing.T) {
@@ -305,7 +271,10 @@ func TestHasMatchingTraits_KitNoTraitShouldNotBePicked(t
*testing.T) {
},
}
- ok, err := HasMatchingTraits(integrationKitSpec, integration)
+ a := buildKitAction{}
+ a.InjectLogger(log.Log)
+
+ ok, err := a.hasMatchingTraits(context.TODO(), integrationKitSpec,
integration)
assert.Nil(t, err)
assert.False(t, ok)
}
@@ -324,7 +293,7 @@ func TestHasMatchingTraits_KitSameTraitShouldBePicked(t
*testing.T) {
Traits: map[string]v1.TraitSpec{
"builder": test.TraitSpecFromMap(t,
map[string]interface{}{
"enabled": "true",
- "buildTimeProperties": []string{
+ "properties": []string{
"build-key1=build-value1",
},
}),
@@ -345,7 +314,7 @@ func TestHasMatchingTraits_KitSameTraitShouldBePicked(t
*testing.T) {
Traits: map[string]v1.TraitSpec{
"builder": test.TraitSpecFromMap(t,
map[string]interface{}{
"enabled": "true",
- "buildTimeProperties": []string{
+ "properties": []string{
"build-key1=build-value1",
},
}),
@@ -353,7 +322,10 @@ func TestHasMatchingTraits_KitSameTraitShouldBePicked(t
*testing.T) {
},
}
- ok, err := HasMatchingTraits(integrationKitSpec, integration)
+ a := buildKitAction{}
+ a.InjectLogger(log.Log)
+
+ ok, err := a.hasMatchingTraits(context.TODO(), integrationKitSpec,
integration)
assert.Nil(t, err)
assert.True(t, ok)
}
diff --git a/pkg/controller/integration/integration_controller.go
b/pkg/controller/integration/integration_controller.go
index ab91434..0a4df39 100644
--- a/pkg/controller/integration/integration_controller.go
+++ b/pkg/controller/integration/integration_controller.go
@@ -308,7 +308,7 @@ func (r *reconcileIntegration) Reconcile(ctx
context.Context, request reconcile.
NewWaitForBindingsAction(),
NewPlatformSetupAction(),
NewInitializeAction(),
- NewBuildKitAction(),
+ newBuildKitAction(),
NewDeployAction(),
NewMonitorAction(),
NewErrorAction(),
diff --git a/pkg/controller/integration/util.go
b/pkg/controller/integration/util.go
deleted file mode 100644
index 2087bd6..0000000
--- a/pkg/controller/integration/util.go
+++ /dev/null
@@ -1,200 +0,0 @@
-/*
-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 integration
-
-import (
- "context"
- "encoding/json"
-
- "github.com/apache/camel-k/pkg/platform"
- "github.com/pkg/errors"
- k8errors "k8s.io/apimachinery/pkg/api/errors"
-
- "k8s.io/apimachinery/pkg/selection"
- k8sclient "sigs.k8s.io/controller-runtime/pkg/client"
-
- v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
- "github.com/apache/camel-k/pkg/util"
- "github.com/apache/camel-k/pkg/util/controller"
- "github.com/apache/camel-k/pkg/util/kubernetes"
-)
-
-// LookupKitForIntegration --
-func LookupKitForIntegration(ctx context.Context, c k8sclient.Reader,
integration *v1.Integration) (*v1.IntegrationKit, error) {
- if integration.Status.IntegrationKit != nil {
- kit, err := kubernetes.GetIntegrationKit(ctx, c,
integration.Status.IntegrationKit.Name,
integration.Status.IntegrationKit.Namespace)
- if err != nil {
- return nil, errors.Wrapf(err, "unable to find
integration kit %s/%s, %s", integration.Status.IntegrationKit.Namespace,
integration.Status.IntegrationKit.Name, err)
- }
-
- return kit, nil
- }
-
- pl, err := platform.GetCurrent(ctx, c, integration.Namespace)
- if err != nil && !k8errors.IsNotFound(err) {
- return nil, err
- }
-
- options := []k8sclient.ListOption{
-
k8sclient.InNamespace(integration.GetIntegrationKitNamespace(pl)),
- k8sclient.MatchingLabels{
- "camel.apache.org/runtime.version":
integration.Status.RuntimeVersion,
- "camel.apache.org/runtime.provider":
string(integration.Status.RuntimeProvider),
- },
- controller.NewLabelSelector("camel.apache.org/kit.type",
selection.In, []string{
- v1.IntegrationKitTypePlatform,
- v1.IntegrationKitTypeExternal,
- }),
- }
-
- kits := v1.NewIntegrationKitList()
- if err := c.List(ctx, &kits, options...); err != nil {
- return nil, err
- }
-
- for _, kit := range kits.Items {
- kit := kit // pin
-
- if kit.Status.Phase == v1.IntegrationKitPhaseError {
- continue
- }
-
- /*
- TODO: moved to label selector
- if kit.Status.RuntimeVersion !=
integration.Status.RuntimeVersion {
- continue
- }
- if kit.Status.RuntimeProvider !=
integration.Status.RuntimeProvider {
- continue
- }
- */
-
- if kit.Status.Version != integration.Status.Version {
- continue
- }
-
- ideps := len(integration.Status.Dependencies)
- cdeps := len(kit.Spec.Dependencies)
-
- if ideps != cdeps {
- continue
- }
-
- // When a platform kit is created it inherits the traits from
the integrations and as
- // some traits may influence the build thus the artifacts
present on the container image,
- // we need to take traits into account when looking up for
compatible kits.
- //
- // It could also happen that an integration is updated and a
trait is modified, if we do
- // not include traits in the lookup, we may use a kit that does
not have all the
- // characteristics required by the integration.
- //
- // A kit can be used only if it contains a subset of the traits
and related configurations
- // declared on integration.
- match, err := HasMatchingTraits(&kit, integration)
- if err != nil {
- return nil, err
- }
- if !match {
- continue
- }
- if util.StringSliceContains(kit.Spec.Dependencies,
integration.Status.Dependencies) {
- return &kit, nil
- }
- }
-
- return nil, nil
-}
-
-// HasMatchingTraits compare traits defined on kit against those defined on
integration.
-func HasMatchingTraits(kit *v1.IntegrationKit, integration *v1.Integration)
(bool, error) {
- // The kit has no trait, but the integration need some
- if len(kit.Spec.Traits) == 0 && len(integration.Spec.Traits) > 0 {
- return false, nil
- }
- for name, kitTrait := range kit.Spec.Traits {
- intTrait, ok := integration.Spec.Traits[name]
- if !ok {
- // skip it because trait configured on kit is not
defined on integration
- return false, nil
- }
- data, err := json.Marshal(intTrait.Configuration)
- if err != nil {
- return false, err
- }
- intConf := make(map[string]interface{})
- err = json.Unmarshal(data, &intConf)
- if err != nil {
- return false, err
- }
- data, err = json.Marshal(kitTrait.Configuration)
- if err != nil {
- return false, err
- }
- kitConf := make(map[string]interface{})
- err = json.Unmarshal(data, &kitConf)
- if err != nil {
- return false, err
- }
- for ck, cv := range kitConf {
- iv, ok := intConf[ck]
- if !ok {
- // skip it because trait configured on kit has
a value that is not defined
- // in integration trait
- return false, nil
- }
- if !equal(iv, cv) {
- // skip it because trait configured on kit has
a value that differs from
- // the one configured on integration
- return false, nil
- }
- }
- }
-
- return true, nil
-}
-
-// We need to try to perform a slice equality in order to prevent a runtime
panic
-func equal(a, b interface{}) bool {
- aSlice, aOk := a.([]interface{})
- bSlice, bOk := b.([]interface{})
-
- if aOk && bOk {
- // Both are slices
- return sliceEqual(aSlice, bSlice)
- }
-
- if aOk || bOk {
- // One of the 2 is a slice
- return false
- }
-
- // None is a slice
- return a == b
-}
-
-func sliceEqual(a, b []interface{}) bool {
- if len(a) != len(b) {
- return false
- }
- for i, v := range a {
- if v != b[i] {
- return false
- }
- }
- return true
-}
diff --git a/pkg/util/test/trait.go b/pkg/util/test/trait.go
index fca3518..93781ac 100644
--- a/pkg/util/test/trait.go
+++ b/pkg/util/test/trait.go
@@ -26,7 +26,6 @@ import (
v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
)
-// TraitSpecFromMap --
func TraitSpecFromMap(t *testing.T, spec map[string]interface{}) v1.TraitSpec {
var trait v1.TraitSpec
@@ -39,7 +38,6 @@ func TraitSpecFromMap(t *testing.T, spec
map[string]interface{}) v1.TraitSpec {
return trait
}
-// TraitSpecFromMap --
func TraitSpecToMap(t *testing.T, spec v1.TraitSpec) map[string]string {
trait := make(map[string]string)
