Skip to content

Commit

Permalink
chore(ctrl): deprecate annotations trait
Browse files Browse the repository at this point in the history
The previous development was flawed, as it did not do the proper comparison with the configuration coming from annotation to the configuration coming from trait spec.
This commit is supporting the comparison in a proper manner, however, we're deprecating the feature from other resources which are not Pipe as it does not make sense to support.

Ref #5620
  • Loading branch information
squakez committed Jun 14, 2024
1 parent a0d3c3e commit a312deb
Show file tree
Hide file tree
Showing 20 changed files with 293 additions and 221 deletions.
5 changes: 2 additions & 3 deletions e2e/common/misc/pipe_with_image_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ func TestPipeWithImage(t *testing.T) {

g.Eventually(IntegrationGeneration(t, ctx, ns, bindingID)).
Should(gstruct.PointTo(BeNumerically("==", 1)))
g.Eventually(Integration(t, ctx, ns, bindingID)).Should(WithTransform(Annotations, And(
g.Eventually(Integration(t, ctx, ns, bindingID)).Should(WithTransform(Annotations,
HaveKeyWithValue("test", "1"),
HaveKeyWithValue("trait.camel.apache.org/container.image", expectedImage),
)))
))
g.Eventually(IntegrationStatusImage(t, ctx, ns, bindingID)).
Should(Equal(expectedImage))
g.Eventually(IntegrationPodPhase(t, ctx, ns, bindingID), TestTimeoutShort).
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/integration/build_kit.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ kits:
k := &existingKits[i]

action.L.Debug("Comparing existing kit with environment", "env kit", kit.Name, "existing kit", k.Name)
match, err := kitMatches(&kit, k)
match, err := kitMatches(action.client, &kit, k)
if err != nil {
return nil, fmt.Errorf("error occurred matches integration kits with environment for integration %s/%s: %w",
integration.Namespace, integration.Name, err)
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/integration/integration_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler {
)
}

func integrationUpdateFunc(old *v1.Integration, it *v1.Integration) bool {
func integrationUpdateFunc(c client.Client, old *v1.Integration, it *v1.Integration) bool {
// Observe the time to first readiness metric
previous := old.Status.GetCondition(v1.IntegrationConditionReady)
next := it.Status.GetCondition(v1.IntegrationConditionReady)
Expand All @@ -99,7 +99,7 @@ func integrationUpdateFunc(old *v1.Integration, it *v1.Integration) bool {
updateIntegrationPhase(it.Name, string(it.Status.Phase))
// If traits have changed, the reconciliation loop must kick in as
// traits may have impact
sameTraits, err := trait.IntegrationsHaveSameTraits(old, it)
sameTraits, err := trait.IntegrationsHaveSameTraits(c, old, it)
if err != nil {
Log.ForIntegration(it).Error(
err,
Expand Down Expand Up @@ -324,7 +324,7 @@ func add(ctx context.Context, mgr manager.Manager, c client.Client, r reconcile.
return false
}

return integrationUpdateFunc(old, it)
return integrationUpdateFunc(c, old, it)
},
DeleteFunc: func(e event.DeleteEvent) bool {
// Evaluates to false if the object has been confirmed deleted
Expand Down
10 changes: 5 additions & 5 deletions pkg/controller/integration/kits.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ func integrationMatches(ctx context.Context, c client.Client, integration *v1.In
return false, err
}

itc, err := trait.NewSpecTraitsOptionsForIntegrationAndPlatform(integration, pl)
itc, err := trait.NewSpecTraitsOptionsForIntegrationAndPlatform(c, integration, pl)
if err != nil {
return false, err
}
ikc, err := trait.NewSpecTraitsOptionsForIntegrationKit(kit)
ikc, err := trait.NewSpecTraitsOptionsForIntegrationKit(c, kit)
if err != nil {
return false, err
}
Expand Down Expand Up @@ -170,7 +170,7 @@ func statusMatches(integration *v1.Integration, kit *v1.IntegrationKit, ilog *lo
}

// kitMatches returns whether the kit matches with the existing target kit.
func kitMatches(kit *v1.IntegrationKit, target *v1.IntegrationKit) (bool, error) {
func kitMatches(c client.Client, kit *v1.IntegrationKit, target *v1.IntegrationKit) (bool, error) {
version := kit.Status.Version
if version == "" {
// Defaults with the version that is going to be set during the kit initialization
Expand All @@ -184,11 +184,11 @@ func kitMatches(kit *v1.IntegrationKit, target *v1.IntegrationKit) (bool, error)
}

// We cannot have yet the status set
c1, err := trait.NewSpecTraitsOptionsForIntegrationKit(kit)
c1, err := trait.NewSpecTraitsOptionsForIntegrationKit(c, kit)
if err != nil {
return false, err
}
c2, err := trait.NewSpecTraitsOptionsForIntegrationKit(target)
c2, err := trait.NewSpecTraitsOptionsForIntegrationKit(c, target)
if err != nil {
return false, err
}
Expand Down
17 changes: 11 additions & 6 deletions pkg/controller/integration/kits_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1"
traitv1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1/trait"
"github.com/apache/camel-k/v2/pkg/client"

"github.com/apache/camel-k/v2/pkg/trait"
"github.com/apache/camel-k/v2/pkg/util/test"
Expand Down Expand Up @@ -279,7 +280,10 @@ func TestHasMatchingTraits_KitNoTraitShouldNotBePicked(t *testing.T) {
},
}

ok, err := integrationAndKitHaveSameTraits(integration, kit)
c, err := test.NewFakeClient(integration, kit)
require.NoError(t, err)

ok, err := integrationAndKitHaveSameTraits(c, integration, kit)
require.NoError(t, err)
assert.False(t, ok)
}
Expand Down Expand Up @@ -326,8 +330,9 @@ func TestHasMatchingTraits_KitSameTraitShouldBePicked(t *testing.T) {
},
},
}

ok, err := integrationAndKitHaveSameTraits(integration, kit)
c, err := test.NewFakeClient(integration, kit)
require.NoError(t, err)
ok, err := integrationAndKitHaveSameTraits(c, integration, kit)
require.NoError(t, err)
assert.True(t, ok)
}
Expand Down Expand Up @@ -429,12 +434,12 @@ func TestHasNotMatchingSources(t *testing.T) {
assert.False(t, hsm2)
}

func integrationAndKitHaveSameTraits(i1 *v1.Integration, i2 *v1.IntegrationKit) (bool, error) {
itOpts, err := trait.NewSpecTraitsOptionsForIntegration(i1)
func integrationAndKitHaveSameTraits(c client.Client, i1 *v1.Integration, i2 *v1.IntegrationKit) (bool, error) {
itOpts, err := trait.NewSpecTraitsOptionsForIntegration(c, i1)
if err != nil {
return false, err
}
ikOpts, err := trait.NewSpecTraitsOptionsForIntegrationKit(i2)
ikOpts, err := trait.NewSpecTraitsOptionsForIntegrationKit(c, i2)
if err != nil {
return false, err
}
Expand Down
24 changes: 24 additions & 0 deletions pkg/controller/integration/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"reflect"
"strconv"
"strings"

appsv1 "k8s.io/api/apps/v1"
batchv1 "k8s.io/api/batch/v1"
Expand Down Expand Up @@ -131,9 +132,32 @@ func (action *monitorAction) Handle(ctx context.Context, integration *v1.Integra
return integration, err
}

action.checkTraitAnnotationsDeprecatedNotice(integration)

return action.monitorPods(ctx, environment, integration)
}

// Deprecated: to be removed in future versions, when we won't support any longer trait annotations into Integrations.
func (action *monitorAction) checkTraitAnnotationsDeprecatedNotice(integration *v1.Integration) {
if integration.Annotations != nil {
for k := range integration.Annotations {
if strings.HasPrefix(k, v1.TraitAnnotationPrefix) {
integration.Status.SetCondition(
v1.IntegrationConditionType("AnnotationTraitsDeprecated"),
corev1.ConditionTrue,
"DeprecationNotice",
"Annotation traits configuration is deprecated and will be removed soon. Use .spec.traits configuration instead.",
)
action.L.Infof(
"WARN: annotation traits configuration is deprecated and will be removed soon. Use .spec.traits configuration for %s integration instead.",
integration.Name,
)
return
}
}
}
}

func (action *monitorAction) monitorPods(ctx context.Context, environment *trait.Environment, integration *v1.Integration) (*v1.Integration, error) {
controller, err := action.newController(environment, integration)
if err != nil {
Expand Down
26 changes: 26 additions & 0 deletions pkg/controller/integrationkit/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ package integrationkit

import (
"context"
"strings"

v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1"
"github.com/apache/camel-k/v2/pkg/util/digest"
corev1 "k8s.io/api/core/v1"
)

// NewMonitorAction creates a new monitoring handling action for the kit.
Expand Down Expand Up @@ -64,5 +66,29 @@ func (action *monitorAction) Handle(ctx context.Context, kit *v1.IntegrationKit)
return kit, nil
}

action.checkTraitAnnotationsDeprecatedNotice(kit)

return nil, nil
}

// Deprecated: to be removed in future versions, when we won't support any longer trait annotations into IntegrationKits.
func (action *monitorAction) checkTraitAnnotationsDeprecatedNotice(integrationKit *v1.IntegrationKit) {
if integrationKit.Annotations != nil {
for k := range integrationKit.Annotations {
if strings.HasPrefix(k, v1.TraitAnnotationPrefix) {
integrationKit.Status.SetCondition(
v1.IntegrationKitConditionType("AnnotationTraitsDeprecated"),
corev1.ConditionTrue,
"DeprecationNotice",
"Annotation traits configuration is deprecated and will be removed soon. Use .spec.traits configuration instead.",
)

action.L.Infof(
"WARN: annotation traits configuration is deprecated and will be removed soon. Use .spec.traits configuration for %s integration kit instead.",
integrationKit.Name,
)
return
}
}
}
}
23 changes: 23 additions & 0 deletions pkg/controller/integrationplatform/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package integrationplatform
import (
"context"
"fmt"
"strings"

v1 "github.com/apache/camel-k/v2/pkg/apis/camel/v1"
platformutil "github.com/apache/camel-k/v2/pkg/platform"
Expand Down Expand Up @@ -124,6 +125,28 @@ func (action *monitorAction) Handle(ctx context.Context, platform *v1.Integratio
}

platform.Status.Phase = platformPhase
action.checkTraitAnnotationsDeprecatedNotice(platform)

return platform, nil
}

// Deprecated: to be removed in future versions, when we won't support any longer trait annotations into IntegrationPlatforms.
func (action *monitorAction) checkTraitAnnotationsDeprecatedNotice(platform *v1.IntegrationPlatform) {
if platform.Annotations != nil {
for k := range platform.Annotations {
if strings.HasPrefix(k, v1.TraitAnnotationPrefix) {
platform.Status.SetCondition(
v1.IntegrationPlatformConditionType("AnnotationTraitsDeprecated"),
corev1.ConditionTrue,
"DeprecationNotice",
"Annotation traits configuration is deprecated and will be removed soon. Use .spec.traits configuration instead.",
)
action.L.Infof(
"WARN: annotation traits configuration is deprecated and will be removed soon. Use .spec.traits configuration for %s platform instead.",
platform.Name,
)
return
}
}
}
}
12 changes: 6 additions & 6 deletions pkg/controller/kameletbinding/kameletbinding_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import (
// Add creates a new KameletBinding Controller and adds it to the Manager. The Manager will set fields on the Controller
// and Start it when the Manager is Started.
func Add(ctx context.Context, mgr manager.Manager, c client.Client) error {
return add(mgr, newReconciler(mgr, c))
return add(mgr, newReconciler(mgr, c), c)
}

func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler {
Expand All @@ -65,14 +65,14 @@ func newReconciler(mgr manager.Manager, c client.Client) reconcile.Reconciler {
)
}

func add(mgr manager.Manager, r reconcile.Reconciler) error {
c, err := controller.New("kamelet-binding-controller", mgr, controller.Options{Reconciler: r})
func add(mgr manager.Manager, r reconcile.Reconciler, c client.Client) error {
ctrl, err := controller.New("kamelet-binding-controller", mgr, controller.Options{Reconciler: r})
if err != nil {
return err
}

// Watch for changes to primary resource KameletBinding
err = c.Watch(source.Kind(mgr.GetCache(), &v1alpha1.KameletBinding{}),
err = ctrl.Watch(source.Kind(mgr.GetCache(), &v1alpha1.KameletBinding{}),
&handler.EnqueueRequestForObject{},
platform.FilteringFuncs{
UpdateFunc: func(e event.UpdateEvent) bool {
Expand All @@ -87,7 +87,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {

// If traits have changed, the reconciliation loop must kick in as
// traits may have impact
sameTraits, err := trait.KameletBindingsHaveSameTraits(oldKameletBinding, newKameletBinding)
sameTraits, err := trait.KameletBindingsHaveSameTraits(c, oldKameletBinding, newKameletBinding)
if err != nil {
Log.ForKameletBinding(newKameletBinding).Error(
err,
Expand All @@ -114,7 +114,7 @@ func add(mgr manager.Manager, r reconcile.Reconciler) error {
}

// Watch Integration to propagate changes downstream
err = c.Watch(source.Kind(mgr.GetCache(), &v1.Integration{}),
err = ctrl.Watch(source.Kind(mgr.GetCache(), &v1.Integration{}),
handler.EnqueueRequestForOwner(
mgr.GetScheme(),
mgr.GetRESTMapper(),
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/kameletbinding/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func (action *monitorAction) Handle(ctx context.Context, binding *v1alpha1.Kamel
integrationProfileNamespaceChanged := v1.GetIntegrationProfileNamespaceAnnotation(binding) != "" &&
(v1.GetIntegrationProfileNamespaceAnnotation(binding) != v1.GetIntegrationProfileNamespaceAnnotation(&it))

sameTraits, err := trait.IntegrationAndKameletBindingSameTraits(&it, binding)
sameTraits, err := trait.IntegrationAndKameletBindingSameTraits(action.client, &it, binding)
if err != nil {
return nil, err
}
Expand Down
52 changes: 1 addition & 51 deletions pkg/controller/pipe/integration.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"encoding/json"
"fmt"
"sort"
"strings"

k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -88,10 +87,6 @@ func CreateIntegrationFor(ctx context.Context, c client.Client, binding *v1.Pipe
it.Spec = *binding.Spec.Integration.DeepCopy()
}

if &it.Spec != nil && traits != nil {
it.Spec = v1.IntegrationSpec{}
}

if traits != nil {
it.Spec.Traits = *traits
}
Expand Down Expand Up @@ -226,52 +221,7 @@ func CreateIntegrationFor(ctx context.Context, c client.Client, binding *v1.Pipe

// extractAndDeleteTraits will extract the annotation traits into v1.Traits struct, removing from the value from the input map.
func extractAndDeleteTraits(c client.Client, annotations map[string]string) (*v1.Traits, error) {
// structure that will be marshalled into a v1.Traits as it was a kamel run command
catalog := trait.NewCatalog(c)
traitsPlainParams := []string{}
for k, v := range annotations {
if strings.HasPrefix(k, v1.TraitAnnotationPrefix) {
key := strings.ReplaceAll(k, v1.TraitAnnotationPrefix, "")
traitId := strings.Split(key, ".")[0]
if err := trait.ValidateTrait(catalog, traitId); err != nil {
return nil, err
}
traitArrayParams := extractAsArray(v)
for _, param := range traitArrayParams {
traitsPlainParams = append(traitsPlainParams, fmt.Sprintf("%s=%s", key, param))
}
delete(annotations, k)
}
}
if len(traitsPlainParams) == 0 {
return nil, nil
}
var traits v1.Traits
if err := trait.ConfigureTraits(traitsPlainParams, &traits, catalog); err != nil {
return nil, err
}

return &traits, nil
}

// extractTraitValue can detect if the value is an array representation as ["prop1=1", "prop2=2"] and
// return an array with the values or with the single value passed as a parameter.
func extractAsArray(value string) []string {
if strings.HasPrefix(value, "[") && strings.HasSuffix(value, "]") {
arrayValue := []string{}
data := value[1 : len(value)-1]
vals := strings.Split(data, ",")
for _, v := range vals {
prop := strings.Trim(v, " ")
if strings.HasPrefix(prop, `"`) && strings.HasSuffix(prop, `"`) {
prop = prop[1 : len(prop)-1]
}
arrayValue = append(arrayValue, prop)
}
return arrayValue
}

return []string{value}
return trait.ExtractAndMaybeDeleteTraits(c, annotations, true)
}

func configureBinding(integration *v1.Integration, bindings ...*bindings.Binding) error {
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/pipe/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (action *monitorAction) Handle(ctx context.Context, pipe *v1.Pipe) (*v1.Pip
integrationProfileNamespaceChanged := v1.GetIntegrationProfileNamespaceAnnotation(pipe) != "" &&
(v1.GetIntegrationProfileNamespaceAnnotation(pipe) != v1.GetIntegrationProfileNamespaceAnnotation(&it))

sameTraits, err := trait.IntegrationAndPipeSameTraits(&it, pipe)
sameTraits, err := trait.IntegrationAndPipeSameTraits(action.client, &it, pipe)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit a312deb

Please sign in to comment.