Skip to content
This repository was archived by the owner on Aug 26, 2025. It is now read-only.

Remove old scheduler configs #780

Merged
tfussell merged 9 commits into from
Nov 24, 2020
Merged

Remove old scheduler configs #780

tfussell merged 9 commits into from
Nov 24, 2020

Conversation

@tfussell
Copy link
Contributor

@tfussell tfussell commented Jul 30, 2020 •
edited
Loading

Towards https://github.com/giantswarm/giantswarm/issues/12349. Kubernetes 1.16 adds strict decoding of the scheduler config which surfaced that we are setting some fields removed in Kubernetes 1.13. This leads to a warning and slightly less safe config loading. I'm deleting the deleted fields and those where the value matches the default.

Checklist

  • Update changelog in CHANGELOG.md.

@tfussell tfussell self-assigned this Jul 30, 2020
tfussell
tfussell commented Jul 30, 2020
View reviewed changes
files/config/scheduler.yaml
clientConnection:
kubeconfig: /etc/kubernetes/kubeconfig/scheduler.yaml
failureDomains: kubernetes.io/hostname,failure-domain.beta.kubernetes.io/zone,failure-domain.beta.kubernetes.io/region
hardPodAffinitySymmetricWeight: 1
Copy link
Contributor Author

@tfussell tfussell Jul 30, 2020

Choose a reason for hiding this comment

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

Moved to scheduler policy config in kubernetes/kubernetes#44159

tfussell
tfussell commented Jul 30, 2020
View reviewed changes
files/config/scheduler.yaml
kind: KubeSchedulerConfiguration
clientConnection:
kubeconfig: /etc/kubernetes/kubeconfig/scheduler.yaml
failureDomains: kubernetes.io/hostname,failure-domain.beta.kubernetes.io/zone,failure-domain.beta.kubernetes.io/region
Copy link
Contributor Author

@tfussell tfussell Jul 30, 2020

Choose a reason for hiding this comment

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

Removed in k8s 1.13 in change from componentconfig/v1alpha1 to kubescheduler.config.k8s.io/v1alpha1.

tfussell
tfussell commented Jul 30, 2020
View reviewed changes
files/config/scheduler.yaml
@@ -1,8 +1,4 @@
kind: KubeSchedulerConfiguration
algorithmSource:
Copy link
Contributor Author

@tfussell tfussell Jul 30, 2020

Choose a reason for hiding this comment

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

Deprecated, defaults to DefaultProvider.

@tfussell tfussell mentioned this pull request Nov 10, 2020
Closed
@tfussell
Copy link
Contributor Author

tfussell commented Nov 24, 2020

Tested on geckon. Before change:

I1124 12:16:58.250247       1 serving.go:312] Generated self-signed cert in-memory
W1124 12:16:58.255259       1 configfile.go:65] using lenient decoding as strict decoding failed: strict decoder error for kind: KubeSchedulerConfiguration
algorithmSource:
  provider: DefaultProvider
apiVersion: kubescheduler.config.k8s.io/v1alpha1
clientConnection:
  kubeconfig: /etc/kubernetes/kubeconfig/scheduler.yaml
failureDomains: kubernetes.io/hostname,failure-domain.beta.kubernetes.io/zone,failure-domain.beta.kubernetes.io/region
hardPodAffinitySymmetricWeight: 1
: v1alpha1.KubeSchedulerConfiguration.HardPodAffinitySymmetricWeight: ReadObject: found unknown field: failureDomains, error found in #10 byte of ...|reDomains":"kubernet|..., bigger context ...|netes/kubeconfig/scheduler.yaml"},"failureDomains":"kubernetes.io/hostname,failure-domain.beta.kuber|...
W1124 12:16:58.866815       1 authentication.go:267] No authentication-kubeconfig provided in order to lookup client-ca-file in configmap/extension-apiserver-authentication in kube-system, so client certificate authentication won't work.
W1124 12:16:58.872418       1 authentication.go:291] No authentication-kubeconfig provided in order to lookup requestheader-client-ca-file in configmap/extension-apiserver-authentication in kube-system, so request-header client certificate authentication won't work.

after change:

I1124 14:33:02.274304       1 flags.go:33] FLAG: --write-config-to=""
I1124 14:33:03.554689       1 serving.go:312] Generated self-signed cert in-memory
W1124 14:33:05.327813       1 authentication.go:267] No authentication-kubeconfig provided in order to lookup client-ca-file in configmap/extension-apiserver-authentication in kube-system, so client certificate authentication won't work.
W1124 14:33:05.327884       1 authentication.go:291] No authentication-kubeconfig provided in order to lookup requestheader-client-ca-file in configmap/extension-apiserver-authentication in kube-system, so request-header client certificate authentication won't work.

@tfussell tfussell marked this pull request as ready for review November 24, 2020 14:48
@tfussell tfussell requested a review from a team November 24, 2020 14:49
calvix
calvix approved these changes Nov 24, 2020
View reviewed changes
Copy link
Contributor

@calvix calvix left a comment

Choose a reason for hiding this comment

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

lgtm

@tfussell tfussell merged commit 323176f into master Nov 24, 2020
@tfussell tfussell deleted the remove-old-scheduler-configs branch November 24, 2020 15:25
@yulianedyalkova
Copy link
Contributor

yulianedyalkova commented Nov 24, 2020

Did we made sure that the behaviour that those flags were defining is preserved (by other flags/options)?

@tfussell
Copy link
Contributor Author

tfussell commented Nov 24, 2020

Did we made sure that the behaviour that those flags were defining is preserved (by other flags/options)?

I think we can tell from the code and the warning message (found unknown field: failureDomains, error found in #10 byte of ...|reDomains":"kubernet) that the config here wasn't being used. It's worth looking into whether we should be setting this on a global or pod-specific level. I think the idea is to use the built-in requiredDuringSchedulingIgnoredDuringExecution option for nodeAffinity on pods when needed.

@yulianedyalkova
Copy link
Contributor

yulianedyalkova commented Nov 24, 2020

I'm not questioning this PR, I'm just assuming we added those flags for a reason in the first place. And it would be nice to eventually evaluate if we still care about them and if there are alternative ways to ensure the behaviour stays the same :) I have no idea what the flags are doing though so I could be talking nonsense 🤷‍♀️

@tfussell
Copy link
Contributor Author

tfussell commented Nov 24, 2020

I'm not questioning this PR, I'm just assuming we added those flags for a reason in the first place. And it would be nice to eventually evaluate if we still care about them and if there are alternative ways to ensure the behaviour stays the same :) I have no idea what the flags are doing though so I could be talking nonsense 🤷‍♀️

For sure. These were deprecated several years ago so it's hard to tell what they did exactly. I tried reading through the Kubernetes git history, but they don't talk about why, only what. My guess is that the scheduler used to handle failure domains in a special way from other affinity/anti-affinity (if affinity even existed back then) but it has been subsumed by new features.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@calvix calvix calvix approved these changes

@MarcelMue MarcelMue MarcelMue approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@tfussell tfussell

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@tfussell @yulianedyalkova @calvix @MarcelMue