Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option to ignore namespaces #6788

Merged
merged 25 commits into from
Jul 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e23e769
gcp: fix unsafe webhook vpa-webhook-config
britslampe Jan 8, 2024
b5bdba3
Fix operator value
britslampe Jan 9, 2024
81ab072
wrap namespaceSelector values for admission-controller with a flag
britslampe Mar 20, 2024
6474581
Merge branch 'master' into master
britslampe Mar 20, 2024
a5f6d01
Merge remote-tracking branch 'britdm/master' into vpa-ignore-namespace
adrianmoisey May 1, 2024
2d38ced
Add "ignored-vpa-object-namespaces"
adrianmoisey May 1, 2024
cfb625e
Rename the option in the admission-controller for consistency
adrianmoisey May 1, 2024
3e4cc78
Move README comment to the toplevel readme
adrianmoisey May 1, 2024
5cca0cc
Add the new example to the index
adrianmoisey May 1, 2024
9e9ca33
Don't pass around pointers
adrianmoisey May 1, 2024
590b4fc
Move error log to below startup log
adrianmoisey May 1, 2024
cef6dcb
Make namespaceSelector empty when the list of namespaces is empty
adrianmoisey May 2, 2024
4073194
Allow webhook to select namespaces
adrianmoisey May 14, 2024
af33e13
Use build in 'contains'
adrianmoisey Jun 7, 2024
8d2bbb8
Merge master
adrianmoisey Jun 20, 2024
1b36617
Add tests
adrianmoisey Jun 26, 2024
6b73976
Merge master
adrianmoisey Jun 26, 2024
ee9d626
Use slices.Contains
adrianmoisey Jun 26, 2024
a8a16a4
Be more explicit about the flags
adrianmoisey Jun 26, 2024
7a1aea1
Pass around comma separate strings rather than lists
adrianmoisey Jun 26, 2024
e14db24
selectedNamespace contains a single namespace
adrianmoisey Jun 29, 2024
a7f2c93
Revert "Pass around comma separate strings rather than lists"
adrianmoisey Jul 2, 2024
7e19ebe
Revert unintentional change
adrianmoisey Jul 2, 2024
edc8091
Remove hardcoded 10 second delay
adrianmoisey Jul 2, 2024
74e7c5f
Merge master
adrianmoisey Jul 10, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions vertical-pod-autoscaler/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- [Starting multiple recommenders](#starting-multiple-recommenders)
- [Using CPU management with static policy](#using-cpu-management-with-static-policy)
- [Controlling eviction behavior based on scaling direction and resource](#controlling-eviction-behavior-based-on-scaling-direction-and-resource)
- [Limiting which namespaces are used](#limiting-which-namespaces-are-used)
- [Known limitations](#known-limitations)
- [Related links](#related-links)

Expand Down Expand Up @@ -376,6 +377,16 @@ vpa-post-processor.kubernetes.io/{containerName}_integerCPU=true
```
Note that this doesn't prevent scaling down entirely, as Pods may get recreated for different reasons, resulting in a new recommendation being applied. See [the original AEP](https://github.com/kubernetes/autoscaler/tree/master/vertical-pod-autoscaler/enhancements/4831-control-eviction-behavior) for more context and usage information.

### Limiting which namespaces are used

By default the VPA will run against all namespaces. You can limit that behaviour by setting the following options:

1. `ignored-vpa-object-namespaces` - A comma separated list of namespaces to ignore
1. `vpa-object-namespace` - A single namespace to monitor

These options cannot be used together and are mutually exclusive.


# Known limitations

* Whenever VPA updates the pod resources, the pod is recreated, which causes all
Expand Down
36 changes: 30 additions & 6 deletions vertical-pod-autoscaler/pkg/admission-controller/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ func configTLS(cfg certsConfig, minTlsVersion, ciphers string, stop <-chan struc

// register this webhook admission controller with the kube-apiserver
// by creating MutatingWebhookConfiguration.
func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32) {
time.Sleep(10 * time.Second)
func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDelay time.Duration, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32, selectedNamespace string, ignoredNamespaces []string) {
time.Sleep(webHookDelay)
client := clientset.AdmissionregistrationV1().MutatingWebhookConfigurations()
_, err := client.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
if err == nil {
Expand All @@ -111,6 +111,29 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace,
sideEffects := admissionregistration.SideEffectClassNone
failurePolicy := admissionregistration.Ignore
RegisterClientConfig.CABundle = caCert

var namespaceSelector metav1.LabelSelector
if len(ignoredNamespaces) > 0 {
namespaceSelector = metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpNotIn,
Values: ignoredNamespaces,
},
},
}
} else if len(selectedNamespace) > 0 {
namespaceSelector = metav1.LabelSelector{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpIn,
Values: []string{selectedNamespace},
},
},
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also be setting a nameSpaceSelector for the case where vpa-object-namespace is used to cause the webhook to operate only for that namespace? This way the pods for all other namespaces do not have to wait for an answer from the admission controller before they start (it will also reduce the load on the admission controller)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!
Fixed in 4073194

webhookConfig := &admissionregistration.MutatingWebhookConfiguration{
ObjectMeta: metav1.ObjectMeta{
Name: webhookConfigName,
Expand All @@ -137,10 +160,11 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace,
},
},
},
FailurePolicy: &failurePolicy,
ClientConfig: RegisterClientConfig,
SideEffects: &sideEffects,
TimeoutSeconds: &timeoutSeconds,
FailurePolicy: &failurePolicy,
ClientConfig: RegisterClientConfig,
SideEffects: &sideEffects,
TimeoutSeconds: &timeoutSeconds,
NamespaceSelector: &namespaceSelector,
},
},
}
Expand Down
79 changes: 76 additions & 3 deletions vertical-pod-autoscaler/pkg/admission-controller/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package main
import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"
admissionregistration "k8s.io/api/admissionregistration/v1"
Expand All @@ -30,13 +31,16 @@ func TestSelfRegistrationBase(t *testing.T) {

testClientSet := fake.NewSimpleClientset()
caCert := []byte("fake")
webHookDelay := 0 * time.Second
namespace := "default"
serviceName := "vpa-service"
url := "http://example.com/"
registerByURL := true
timeoutSeconds := int32(32)
selectedNamespace := ""
ignoredNamespaces := []string{}

selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds)
selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)

webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
Expand Down Expand Up @@ -70,13 +74,16 @@ func TestSelfRegistrationWithURL(t *testing.T) {

testClientSet := fake.NewSimpleClientset()
caCert := []byte("fake")
webHookDelay := 0 * time.Second
namespace := "default"
serviceName := "vpa-service"
url := "http://example.com/"
registerByURL := true
timeoutSeconds := int32(32)
selectedNamespace := ""
ignoredNamespaces := []string{}

selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds)
selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)

webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
Expand All @@ -95,13 +102,16 @@ func TestSelfRegistrationWithOutURL(t *testing.T) {

testClientSet := fake.NewSimpleClientset()
caCert := []byte("fake")
webHookDelay := 0 * time.Second
namespace := "default"
serviceName := "vpa-service"
url := "http://example.com/"
registerByURL := false
timeoutSeconds := int32(32)
selectedNamespace := ""
ignoredNamespaces := []string{}

selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds)
selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)

webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
Expand All @@ -117,3 +127,66 @@ func TestSelfRegistrationWithOutURL(t *testing.T) {

assert.Nil(t, webhook.ClientConfig.URL, "expected URL to be set")
}

func TestSelfRegistrationWithIgnoredNamespaces(t *testing.T) {

testClientSet := fake.NewSimpleClientset()
caCert := []byte("fake")
webHookDelay := 0 * time.Second
namespace := "default"
serviceName := "vpa-service"
url := "http://example.com/"
registerByURL := false
timeoutSeconds := int32(32)
selectedNamespace := ""
ignoredNamespaces := []string{"test"}

selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)

webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})

assert.NoError(t, err, "expected no error fetching webhook configuration")

assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration")
webhook := webhookConfig.Webhooks[0]

assert.NotNil(t, webhook.NamespaceSelector.MatchExpressions, "expected namespace selector not to be nil")
assert.Len(t, webhook.NamespaceSelector.MatchExpressions, 1, "expected one match expression")

matchExpression := webhook.NamespaceSelector.MatchExpressions[0]
assert.Equal(t, matchExpression.Operator, metav1.LabelSelectorOpNotIn, "expected namespace operator to be OpNotIn")
assert.Equal(t, matchExpression.Values, ignoredNamespaces, "expected namespace selector match expression to be equal")
}

func TestSelfRegistrationWithSelectedNamespaces(t *testing.T) {

testClientSet := fake.NewSimpleClientset()
caCert := []byte("fake")
webHookDelay := 0 * time.Second
namespace := "default"
serviceName := "vpa-service"
url := "http://example.com/"
registerByURL := false
timeoutSeconds := int32(32)
selectedNamespace := "test"
ignoredNamespaces := []string{}

selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)

webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})

assert.NoError(t, err, "expected no error fetching webhook configuration")

assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration")
webhook := webhookConfig.Webhooks[0]

assert.NotNil(t, webhook.NamespaceSelector.MatchExpressions, "expected namespace selector not to be nil")
assert.Len(t, webhook.NamespaceSelector.MatchExpressions, 1, "expected one match expression")

matchExpression := webhook.NamespaceSelector.MatchExpressions[0]
assert.Equal(t, metav1.LabelSelectorOpIn, matchExpression.Operator, "expected namespace operator to be OpIn")
assert.Equal(t, matchExpression.Operator, metav1.LabelSelectorOpIn, "expected namespace operator to be OpIn")
assert.Equal(t, matchExpression.Values, []string{selectedNamespace}, "expected namespace selector match expression to be equal")
}
36 changes: 22 additions & 14 deletions vertical-pod-autoscaler/pkg/admission-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"net/http"
"os"
"strings"
"time"

apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -51,6 +52,7 @@ const (
scaleCacheEntryLifetime time.Duration = time.Hour
scaleCacheEntryFreshnessTime time.Duration = 10 * time.Minute
scaleCacheEntryJitterFactor float64 = 1.
webHookDelay = 10 * time.Second
)

var (
Expand All @@ -63,26 +65,31 @@ var (
ciphers = flag.String("tls-ciphers", "", "A comma-separated or colon-separated list of ciphers to accept. Only works when min-tls-version is set to tls1_2.")
minTlsVersion = flag.String("min-tls-version", "tls1_2", "The minimum TLS version to accept. Must be set to either tls1_2 (default) or tls1_3.")

port = flag.Int("port", 8000, "The port to listen on.")
address = flag.String("address", ":8944", "The address to expose Prometheus metrics.")
kubeconfig = flag.String("kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.")
kubeApiQps = flag.Float64("kube-api-qps", 5.0, `QPS limit when making requests to Kubernetes apiserver`)
kubeApiBurst = flag.Float64("kube-api-burst", 10.0, `QPS burst limit when making requests to Kubernetes apiserver`)
namespace = os.Getenv("NAMESPACE")
serviceName = flag.String("webhook-service", "vpa-webhook", "Kubernetes service under which webhook is registered. Used when registerByURL is set to false.")
webhookAddress = flag.String("webhook-address", "", "Address under which webhook is registered. Used when registerByURL is set to true.")
webhookPort = flag.String("webhook-port", "", "Server Port for Webhook")
webhookTimeout = flag.Int("webhook-timeout-seconds", 30, "Timeout in seconds that the API server should wait for this webhook to respond before failing.")
registerWebhook = flag.Bool("register-webhook", true, "If set to true, admission webhook object will be created on start up to register with the API server.")
registerByURL = flag.Bool("register-by-url", false, "If set to true, admission webhook will be registered by URL (webhookAddress:webhookPort) instead of by service name")
vpaObjectNamespace = flag.String("vpa-object-namespace", apiv1.NamespaceAll, "Namespace to search for VPA objects. Empty means all namespaces will be used.")
port = flag.Int("port", 8000, "The port to listen on.")
address = flag.String("address", ":8944", "The address to expose Prometheus metrics.")
kubeconfig = flag.String("kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.")
kubeApiQps = flag.Float64("kube-api-qps", 5.0, `QPS limit when making requests to Kubernetes apiserver`)
kubeApiBurst = flag.Float64("kube-api-burst", 10.0, `QPS burst limit when making requests to Kubernetes apiserver`)
namespace = os.Getenv("NAMESPACE")
serviceName = flag.String("webhook-service", "vpa-webhook", "Kubernetes service under which webhook is registered. Used when registerByURL is set to false.")
webhookAddress = flag.String("webhook-address", "", "Address under which webhook is registered. Used when registerByURL is set to true.")
webhookPort = flag.String("webhook-port", "", "Server Port for Webhook")
webhookTimeout = flag.Int("webhook-timeout-seconds", 30, "Timeout in seconds that the API server should wait for this webhook to respond before failing.")
registerWebhook = flag.Bool("register-webhook", true, "If set to true, admission webhook object will be created on start up to register with the API server.")
registerByURL = flag.Bool("register-by-url", false, "If set to true, admission webhook will be registered by URL (webhookAddress:webhookPort) instead of by service name")
vpaObjectNamespace = flag.String("vpa-object-namespace", apiv1.NamespaceAll, "Namespace to search for VPA objects. Empty means all namespaces will be used. Must not be used if ignored-vpa-object-namespaces is set.")
ignoredVpaObjectNamespaces = flag.String("ignored-vpa-object-namespaces", "", "Comma separated list of namespaces to ignore. Must not be used if vpa-object-namespace is used.")
)

func main() {
klog.InitFlags(nil)
kube_flag.InitFlags()
klog.V(1).Infof("Vertical Pod Autoscaler %s Admission Controller", common.VerticalPodAutoscalerVersion)

if len(*vpaObjectNamespace) > 0 && len(*ignoredVpaObjectNamespaces) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably happen after the info log below to still give a log about the version used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 590b4fc

klog.Fatalf("--vpa-object-namespace and --ignored-vpa-object-namespaces are mutually exclusive and can't be set together.")
}

healthCheck := metrics.NewHealthCheck(time.Minute)
metrics.Initialize(*address, healthCheck)
metrics_admission.Register()
Expand Down Expand Up @@ -136,9 +143,10 @@ func main() {
TLSConfig: configTLS(*certsConfiguration, *minTlsVersion, *ciphers, stopCh),
}
url := fmt.Sprintf("%v:%v", *webhookAddress, *webhookPort)
ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",")
go func() {
if *registerWebhook {
selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout))
selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), webHookDelay, namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), *vpaObjectNamespace, ignoredNamespaces)
}
// Start status updates after the webhook is initialized.
statusUpdater.Run(stopCh)
Expand Down
10 changes: 10 additions & 0 deletions vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package input
import (
"context"
"fmt"
"slices"
"time"

apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -87,6 +88,7 @@ type ClusterStateFeederFactory struct {
MemorySaveMode bool
ControllerFetcher controllerfetcher.ControllerFetcher
RecommenderName string
IgnoredNamespaces []string
}

// Make creates new ClusterStateFeeder with internal data providers, based on kube client.
Expand All @@ -103,6 +105,7 @@ func (m ClusterStateFeederFactory) Make() *clusterStateFeeder {
memorySaveMode: m.MemorySaveMode,
controllerFetcher: m.ControllerFetcher,
recommenderName: m.RecommenderName,
ignoredNamespaces: m.IgnoredNamespaces,
}
}

Expand Down Expand Up @@ -192,6 +195,7 @@ type clusterStateFeeder struct {
memorySaveMode bool
controllerFetcher controllerfetcher.ControllerFetcher
recommenderName string
ignoredNamespaces []string
}

func (feeder *clusterStateFeeder) InitFromHistoryProvider(historyProvider history.HistoryProvider) {
Expand Down Expand Up @@ -332,6 +336,12 @@ func filterVPAs(feeder *clusterStateFeeder, allVpaCRDs []*vpa_types.VerticalPodA
continue
}
}

if slices.Contains(feeder.ignoredNamespaces, vpaCRD.ObjectMeta.Namespace) {
klog.V(6).Infof("Ignoring vpaCRD %s in namespace %s as namespace is ignored", vpaCRD.Name, vpaCRD.Namespace)
continue
}

vpaCRDs = append(vpaCRDs, vpaCRD)
}
return vpaCRDs
Expand Down
Loading
Loading