-
Notifications
You must be signed in to change notification settings - Fork 4k
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
New expander: priority expander #1801
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,3 +8,4 @@ cluster_autoscaler | |
*.un~ | ||
Session.vim | ||
.netrwhist | ||
.vscode |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
/* | ||
Copyright 2016 The Kubernetes Authors. | ||
|
||
Licensed 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 priority | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
|
||
kube_errors "k8s.io/apimachinery/pkg/api/errors" | ||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
"k8s.io/apimachinery/pkg/fields" | ||
"k8s.io/apimachinery/pkg/watch" | ||
v1 "k8s.io/client-go/kubernetes/typed/core/v1" | ||
"k8s.io/klog" | ||
api "k8s.io/kubernetes/pkg/apis/core" | ||
) | ||
|
||
const ( | ||
// PriorityConfigMapName defines a name of the ConfigMap used to store priority expander configuration | ||
PriorityConfigMapName = "cluster-autoscaler-priority-expander" | ||
// ConfigMapKey defines the key used in the ConfigMap to configure priorities | ||
ConfigMapKey = "priorities" | ||
) | ||
|
||
// InitPriorityConfigMap initializes ConfigMap with priority expander configurations. It checks if the map exists | ||
// and has the correct top level key. If it doesn't, it returns error or Exits. If the value is found, | ||
// the current value is fetched and a Watcher is started to watch for changes. It returns the current value of | ||
// the config map, the channel with value updates and an error. | ||
func InitPriorityConfigMap(maps v1.ConfigMapInterface, namespace string) (string, <-chan watch.Event, error) { | ||
errMsg := "" | ||
priorities := "" | ||
|
||
configMap, getStatusError := maps.Get(PriorityConfigMapName, metav1.GetOptions{}) | ||
if getStatusError == nil { | ||
priorities = configMap.Data[ConfigMapKey] | ||
} else if kube_errors.IsNotFound(getStatusError) { | ||
errMsg = fmt.Sprintf("Priority expander config map %s/%s not found. You have to create it before starting cluster-autoscaler "+ | ||
"with priority expander.", namespace, PriorityConfigMapName) | ||
} else { | ||
errMsg = fmt.Sprintf("Failed to retrieve priority expander configmap %s/%s: %v", namespace, PriorityConfigMapName, | ||
getStatusError) | ||
} | ||
if errMsg != "" { | ||
return "", nil, errors.New(errMsg) | ||
} | ||
|
||
watcher, err := maps.Watch(metav1.ListOptions{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Listers already handle watch cache and all necessary error handling and we use those for all other objects. I don't think we should re-implement this functionality. In particular if you want to use watch directly you need to handle channel closing - in both HPA and VPA we've already run into issues caused by watch channel abruptly closing and we needed to implement a retry logic for this case. I don't see such retry logic in this PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't know about any problems using watch API - when is the channel closed? Can you point me to some docs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know if there is any documentation on the details. IIRC @jbartosik once run into similar issue, so he may provide more details. The general recommendation is to always use higher-level abstractions provided by client-go library, rather than raw client. |
||
FieldSelector: fields.OneTermEqualSelector(api.ObjectNameField, PriorityConfigMapName).String(), | ||
Watch: true, | ||
}) | ||
if err != nil { | ||
errMsg = fmt.Sprintf("Error when starting a watcher for changes of the priority expander configmap %s/%s: %v", | ||
namespace, PriorityConfigMapName, err) | ||
klog.Errorf(errMsg) | ||
return "", nil, errors.New(errMsg) | ||
} | ||
|
||
return priorities, watcher.ResultChan(), nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
apiVersion: v1 | ||
kind: ConfigMap | ||
metadata: | ||
name: cluster-autoscaler-priority-expander | ||
data: | ||
priorities: |- | ||
10: | ||
- .*t2\.large.* | ||
- .*t3\.large.* | ||
50: | ||
- .*m4\.4xlarge.* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once this merges we should also update https://github.com/kubernetes/kubernetes/blob/master/cluster/addons/rbac/cluster-autoscaler/cluster-autoscaler-rbac.yaml#L54.