Skip to content

Commit

Permalink
fix: Restrict Knative service trait to Http exposed and passive endpo…
Browse files Browse the repository at this point in the history
…ints

- Only apply Knative service trait on integrations that either use passive consumers or expos at least one Http endpoint
- Integrations are considered unsuitable for Knative service trait when they exclusively use active consumers that are not Http based
- Do not use Knative service with `min-scale=1` for unsuitable integrations instead use normal deployment
  • Loading branch information
christophd authored and squakez committed May 17, 2023
1 parent 8f74139 commit 636aa1c
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 32 deletions.
35 changes: 4 additions & 31 deletions pkg/trait/knative_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,31 +117,6 @@ func (t *knativeServiceTrait) Configure(e *Environment) (bool, error) {
return false, nil
}

if pointer.BoolDeref(t.Auto, true) {
// Check the right value for minScale, as not all services are allowed to scale down to 0
if t.MinScale == nil {
sources, err := kubernetes.ResolveIntegrationSources(e.Ctx, t.Client, e.Integration, e.Resources)
if err != nil {
e.Integration.Status.SetErrorCondition(
v1.IntegrationConditionKnativeServiceAvailable,
v1.IntegrationConditionKnativeServiceNotAvailableReason,
err,
)

return false, err
}

meta, err := metadata.ExtractAll(e.CamelCatalog, sources)
if err != nil {
return false, err
}
if !meta.ExposesHTTPServices || !meta.PassiveEndpoints {
single := 1
t.MinScale = &single
}
}
}

if e.IntegrationInPhase(v1.IntegrationPhaseRunning, v1.IntegrationPhaseError) {
condition := e.Integration.Status.GetCondition(v1.IntegrationConditionKnativeServiceAvailable)
return condition != nil && condition.Status == corev1.ConditionTrue, nil
Expand All @@ -168,11 +143,8 @@ func (t *knativeServiceTrait) Apply(e *Environment) error {
}

func (t *knativeServiceTrait) SelectControllerStrategy(e *Environment) (*ControllerStrategy, error) {
knativeServiceStrategy := ControllerStrategyKnativeService
if t.Enabled != nil {
if *t.Enabled {
return &knativeServiceStrategy, nil
}
if !pointer.BoolDeref(t.Enabled, true) {
// explicitly disabled
return nil, nil
}

Expand All @@ -186,7 +158,8 @@ func (t *knativeServiceTrait) SelectControllerStrategy(e *Environment) (*Control
if err != nil {
return nil, err
}
if meta.ExposesHTTPServices {
if meta.ExposesHTTPServices || meta.PassiveEndpoints {
knativeServiceStrategy := ControllerStrategyKnativeService
return &knativeServiceStrategy, nil
}
return nil, nil
Expand Down
74 changes: 73 additions & 1 deletion pkg/trait/knative_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"
Expand Down Expand Up @@ -265,7 +266,7 @@ func TestKnativeServiceWithCustomContainerName(t *testing.T) {
)
}

func TestKnativeServiceWithResr(t *testing.T) {
func TestKnativeServiceWithRest(t *testing.T) {
catalog, err := camel.DefaultCatalog()
require.NoError(t, err)

Expand Down Expand Up @@ -339,6 +340,77 @@ func TestKnativeServiceWithResr(t *testing.T) {
}))
}

func TestKnativeServiceNotApplicable(t *testing.T) {
catalog, err := camel.DefaultCatalog()
require.NoError(t, err)

client, _ := test.NewFakeClient()
traitCatalog := NewCatalog(nil)

environment := Environment{
CamelCatalog: catalog,
Catalog: traitCatalog,
Client: client,
Integration: &v1.Integration{
ObjectMeta: metav1.ObjectMeta{
Name: KnativeServiceTestName,
Namespace: KnativeServiceTestNamespace,
},
Status: v1.IntegrationStatus{
Phase: v1.IntegrationPhaseDeploying,
},
Spec: v1.IntegrationSpec{
Profile: v1.TraitProfileKnative,
Sources: []v1.SourceSpec{
{
DataSpec: v1.DataSpec{
Name: "routes.js",
Content: `from("timer:tick").log("hello")`,
},
Language: v1.LanguageJavaScript,
},
},
},
},
IntegrationKit: &v1.IntegrationKit{
Status: v1.IntegrationKitStatus{
Phase: v1.IntegrationKitPhaseReady,
},
},
Platform: &v1.IntegrationPlatform{
Spec: v1.IntegrationPlatformSpec{
Cluster: v1.IntegrationPlatformClusterOpenShift,
Build: v1.IntegrationPlatformBuildSpec{
PublishStrategy: v1.IntegrationPlatformBuildPublishStrategyS2I,
Registry: v1.RegistrySpec{Address: "registry"},
RuntimeVersion: catalog.Runtime.Version,
},
},
Status: v1.IntegrationPlatformStatus{
Phase: v1.IntegrationPlatformPhaseReady,
},
},
EnvVars: make([]corev1.EnvVar, 0),
ExecutedTraits: make([]Trait, 0),
Resources: kubernetes.NewCollection(),
}
environment.Platform.ResyncStatusFullConfig()

err = traitCatalog.apply(&environment)

require.NoError(t, err)
assert.NotEmpty(t, environment.ExecutedTraits)
assert.NotNil(t, environment.GetTrait("knative"))

assert.Nil(t, environment.Resources.GetKnativeService(func(service *serving.Service) bool {
return service.Name == KnativeServiceTestName
}))

assert.NotNil(t, environment.Resources.GetDeployment(func(deployment *appsv1.Deployment) bool {
return deployment.Name == KnativeServiceTestName
}))
}

func TestKnativeServiceWithRollout(t *testing.T) {
environment := createKnativeServiceTestEnvironment(t, &traitv1.KnativeServiceTrait{RolloutDuration: "60s"})
assert.NotEmpty(t, environment.ExecutedTraits)
Expand Down

0 comments on commit 636aa1c

Please # to comment.