Skip to content

Commit

Permalink
Force activator always in path when activator CA is specified
Browse files Browse the repository at this point in the history
As described in the feature doc of #11906,
activator needs to serve TLS traffic for the traffic from ingress.

So, this patch force SKS proxy mode when `activator-ca` in
`config-network` is specified.
  • Loading branch information
nak3 committed Mar 28, 2022
1 parent ad64980 commit 15845a5
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 2 deletions.
4 changes: 4 additions & 0 deletions pkg/reconciler/autoscaling/config/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package config
import (
"context"

network "knative.dev/networking/pkg"
"knative.dev/pkg/configmap"
asconfig "knative.dev/serving/pkg/autoscaler/config"
"knative.dev/serving/pkg/autoscaler/config/autoscalerconfig"
Expand All @@ -31,6 +32,7 @@ type cfgKey struct{}
type Config struct {
Autoscaler *autoscalerconfig.Config
Deployment *deployment.Config
Network *network.Config
}

// FromContext fetch config from context.
Expand Down Expand Up @@ -65,6 +67,7 @@ func NewStore(logger configmap.Logger, onAfterStore ...func(name string, value i
configmap.Constructors{
asconfig.ConfigName: asconfig.NewConfigFromConfigMap,
deployment.ConfigName: deployment.NewConfigFromConfigMap,
network.ConfigName: network.NewConfigFromConfigMap,
},
onAfterStore...,
),
Expand All @@ -82,5 +85,6 @@ func (s *Store) Load() *Config {
return &Config{
Autoscaler: s.UntypedLoad(asconfig.ConfigName).(*autoscalerconfig.Config).DeepCopy(),
Deployment: s.UntypedLoad(deployment.ConfigName).(*deployment.Config).DeepCopy(),
Network: s.UntypedLoad(network.ConfigName).(*network.Config).DeepCopy(),
}
}
15 changes: 14 additions & 1 deletion pkg/reconciler/autoscaling/config/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/google/go-cmp/cmp"
logtesting "knative.dev/pkg/logging/testing"

network "knative.dev/networking/pkg"
. "knative.dev/pkg/configmap/testing"
autoscalerconfig "knative.dev/serving/pkg/autoscaler/config"
"knative.dev/serving/pkg/deployment"
Expand All @@ -34,8 +35,10 @@ func TestStoreLoadWithContext(t *testing.T) {

autoscalerConfig := ConfigMapFromTestFile(t, autoscalerconfig.ConfigName)
depConfig := ConfigMapFromTestFile(t, deployment.ConfigName, deployment.DeprecatedQueueSidecarImageKey)
netConfig := ConfigMapFromTestFile(t, network.ConfigName)
store.OnConfigChanged(autoscalerConfig)
store.OnConfigChanged(depConfig)
store.OnConfigChanged(netConfig)
config := FromContext(store.ToContext(context.Background()))

wantAS, _ := autoscalerconfig.NewConfigFromConfigMap(autoscalerConfig)
Expand All @@ -46,6 +49,10 @@ func TestStoreLoadWithContext(t *testing.T) {
if !cmp.Equal(wantD, config.Deployment) {
t.Error("Deployment ConfigMap mismatch (-want, +got):", cmp.Diff(wantD, config.Deployment))
}
wantNet, _ := network.NewConfigFromConfigMap(netConfig)
if !cmp.Equal(wantNet, config.Network) {
t.Error("Network ConfigMap mismatch (-want, +got):", cmp.Diff(wantNet, config.Network))
}
}

func TestStoreImmutableConfig(t *testing.T) {
Expand All @@ -54,16 +61,22 @@ func TestStoreImmutableConfig(t *testing.T) {
store.OnConfigChanged(ConfigMapFromTestFile(t, autoscalerconfig.ConfigName))
store.OnConfigChanged(ConfigMapFromTestFile(t, deployment.ConfigName,
deployment.DeprecatedQueueSidecarImageKey))
store.OnConfigChanged(ConfigMapFromTestFile(t, network.ConfigName))

config := store.Load()
config.Autoscaler.MaxScaleUpRate = 100.0
config.Deployment.ProgressDeadline = 3 * time.Minute
config.Network.ActivatorCA = "activator-ca"
config.Network.ActivatorSAN = "activator-san"
newConfig := store.Load()

if newConfig.Autoscaler.MaxScaleUpRate == 100.0 {
t.Error("Autoscaler config is not immuable")
}
if newConfig.Deployment.ProgressDeadline == 3*time.Minute {
t.Error("Autoscaler config is not immuable")
t.Error("Deployment config is not immuable")
}
if newConfig.Network.ActivatorCA == "activator-ca" || newConfig.Network.ActivatorSAN == "activator-san" {
t.Error("Network config is not immuable")
}
}
8 changes: 8 additions & 0 deletions pkg/reconciler/autoscaling/hpa/hpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import (
"k8s.io/apimachinery/pkg/types"
ktesting "k8s.io/client-go/testing"

netpkg "knative.dev/networking/pkg"
"knative.dev/networking/pkg/apis/networking"
nv1a1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
"knative.dev/pkg/configmap"
Expand Down Expand Up @@ -93,6 +94,13 @@ func TestControllerCanReconcile(t *testing.T) {
Data: map[string]string{
deployment.QueueSidecarImageKey: "motorbike-sidecar",
},
},
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: netpkg.ConfigName,
},
Data: map[string]string{},
}))

waitInformers, err := RunAndSyncInformers(ctx, infs...)
Expand Down
6 changes: 5 additions & 1 deletion pkg/reconciler/autoscaling/kpa/kpa.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,11 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, pa *autoscalingv1alpha1.
// this revision, e.g. after a restart) but PA status is inactive (it was
// already scaled to 0).
// 2. The excess burst capacity is negative.
if want == 0 || decider.Status.ExcessBurstCapacity < 0 || want == scaleUnknown && pa.Status.IsInactive() {
if len(config.FromContext(ctx).Network.ActivatorCA) > 0 {
// When activator CA is enabled, foce activator always in path.
// See: https://github.com/knative/serving/issues/11906
mode = nv1alpha1.SKSOperationModeProxy
} else if want == 0 || decider.Status.ExcessBurstCapacity < 0 || want == scaleUnknown && pa.Status.IsInactive() {
mode = nv1alpha1.SKSOperationModeProxy
}

Expand Down
64 changes: 64 additions & 0 deletions pkg/reconciler/autoscaling/kpa/kpa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import (
"go.uber.org/atomic"
"golang.org/x/sync/errgroup"

netpkg "knative.dev/networking/pkg"
nv1a1 "knative.dev/networking/pkg/apis/networking/v1alpha1"
duckv1 "knative.dev/pkg/apis/duck/v1"
"knative.dev/pkg/configmap"
Expand Down Expand Up @@ -125,15 +126,29 @@ func initialScaleZeroASConfig() *autoscalerconfig.Config {
return ac
}

func activatorCertsNetConfig() *netpkg.Config {
nc, _ := netpkg.NewConfigFromMap(map[string]string{
netpkg.ActivatorCAKey: "knative-ca",
netpkg.ActivatorSANKey: "knative-san",
})
return nc
}

func defaultConfig() *config.Config {
ac, _ := asconfig.NewConfigFromMap(defaultConfigMapData())
deploymentConfig, _ := deployment.NewConfigFromMap(map[string]string{
deployment.QueueSidecarImageKey: "bob",
deployment.ProgressDeadlineKey: progressDeadline.String(),
})
networkConfig, _ := netpkg.NewConfigFromMap(map[string]string{
netpkg.ActivatorCAKey: "",
netpkg.ActivatorSANKey: "",
})

return &config.Config{
Autoscaler: ac,
Deployment: deploymentConfig,
Network: networkConfig,
}
}

Expand All @@ -152,6 +167,12 @@ func newConfigWatcher() configmap.Watcher {
Data: map[string]string{
deployment.QueueSidecarImageKey: "covid is here",
},
}, &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Namespace: system.Namespace(),
Name: netpkg.ConfigName,
},
Data: map[string]string{},
})
}

Expand Down Expand Up @@ -293,6 +314,7 @@ func TestReconcile(t *testing.T) {

type deciderKey struct{}
type asConfigKey struct{}
type netConfigKey struct{}

retryAttempted := false

Expand Down Expand Up @@ -1120,6 +1142,38 @@ func TestReconcile(t *testing.T) {
WithPAMetricsService(privateSvc), WithObservedGeneration(1),
),
}},
}, {
Name: "we have enough burst capacity, but keep proxy mode as activator CA is enabled",
Key: key,
Ctx: context.WithValue(context.WithValue(context.Background(), netConfigKey{}, activatorCertsNetConfig()), deciderKey{},
decider(testNamespace, testRevision, defaultScale, /* desiredScale */
1 /* ebc */)),
Objects: []runtime.Object{
kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized,
WithPAMetricsService(privateSvc), withScales(1, defaultScale),
WithPAStatusService(testRevision), WithObservedGeneration(1)),
defaultProxySKS,
metric(testNamespace, testRevision),
defaultDeployment,
defaultReady},
// No update from ProxySKS.
}, {
Name: "we have enough burst capacity, but switch to keep proxy mode as activator CA is turned on",
Key: key,
Ctx: context.WithValue(context.WithValue(context.Background(), netConfigKey{}, activatorCertsNetConfig()), deciderKey{},
decider(testNamespace, testRevision, defaultScale, /* desiredScale */
1 /* ebc */)),
Objects: []runtime.Object{
kpa(testNamespace, testRevision, WithPASKSReady, WithTraffic, markScaleTargetInitialized,
WithPAMetricsService(privateSvc), withScales(1, defaultScale),
WithPAStatusService(testRevision), WithObservedGeneration(1)),
defaultSKS,
metric(testNamespace, testRevision),
defaultDeployment,
defaultReady},
WantUpdates: []clientgotesting.UpdateActionImpl{{
Object: defaultProxySKS,
}},
}}

table.Test(t, MakeFactory(func(ctx context.Context, listers *Listers, cmw configmap.Watcher) controller.Reconciler {
Expand All @@ -1145,6 +1199,9 @@ func TestReconcile(t *testing.T) {
if asConfig := ctx.Value(asConfigKey{}); asConfig != nil {
testConfigs.Autoscaler = asConfig.(*autoscalerconfig.Config)
}
if netConfig := ctx.Value(netConfigKey{}); netConfig != nil {
testConfigs.Network = netConfig.(*netpkg.Config)
}
psf := podscalable.Get(ctx)
scaler := newScaler(ctx, psf, func(interface{}, time.Duration) {})
scaler.activatorProbe = func(*autoscalingv1alpha1.PodAutoscaler, http.RoundTripper) (bool, error) { return true, nil }
Expand Down Expand Up @@ -1219,6 +1276,13 @@ func TestGlobalResyncOnUpdateAutoscalerConfigMap(t *testing.T) {
deployment.QueueSidecarImageKey: "i'm on a bike",
},
})
watcher.OnChange(&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: netpkg.ConfigName,
Namespace: system.Namespace(),
},
Data: map[string]string{},
})

grp := errgroup.Group{}
waitInformers, err := RunAndSyncInformers(ctx, informers...)
Expand Down

0 comments on commit 15845a5

Please sign in to comment.