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 configmap in openshift-config namespace and watch it #57

Merged

Conversation

ravisantoshgudimetla
Copy link
Contributor

At a high level, following is what this PR does:

  • When cluster-admin creates a configmap with name policy-configmap in openshift-config namespace, the kube-scheduler operator creates a new configmap with same name in openshift-kube-scheduler namespace.
  • It continuously watches for configmap in openshift-configmap namespace, when changes are made to configmap in openshift-configmap, the same changes are made to configmap in openshift-kube-scheduler operator.
  • When configmap is deleted openshift-configmap namespace, the corresponding configmap is deleted and scheduler falls back to default provider.

/cc @sjenning

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 2, 2019
@ravisantoshgudimetla
Copy link
Contributor Author

/retest

@ravisantoshgudimetla
Copy link
Contributor Author

@deads2k - Is it ok to revert to old revision of a config when the current revision fails for x minutes or something similar to that? Do we have logic for it currently?

@ravisantoshgudimetla
Copy link
Contributor Author

/retest

6 similar comments
@ravisantoshgudimetla
Copy link
Contributor Author

/retest

@sjenning
Copy link
Contributor

sjenning commented Feb 5, 2019

/retest

@ravisantoshgudimetla
Copy link
Contributor Author

/retest

@ravisantoshgudimetla
Copy link
Contributor Author

/retest

@openshift-merge-robot
Copy link
Contributor

/retest

@ravisantoshgudimetla
Copy link
Contributor Author

/retest

@sjenning
Copy link
Contributor

sjenning commented Feb 7, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 7, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ravisantoshgudimetla, sjenning

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [ravisantoshgudimetla,sjenning]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

kind: KubeSchedulerConfiguration
clientConnection:
kubeconfig: /etc/kubernetes/static-pod-resources/secrets/scheduler-kubeconfig/kubeconfig
algorithmSource:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we installing default policies in some environments? For example - it is pointless to enable Cinder, Azure, GCE-PD predicates in Openshift-4.0 environment on top of AWS?

Copy link
Contributor

@sjenning sjenning Feb 7, 2019

Choose a reason for hiding this comment

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

it is pointless but doesn't hurt anything and, if we can use the same default config for all platforms, then it is worth the reduced complexity in my mind.

Copy link
Member

@gnufied gnufied Feb 7, 2019

Choose a reason for hiding this comment

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

There is a slight performance improvement for sure if you disable the unneeded predicates.
I am looking to enable a Cinder specific predicate which is disabled by default in Openshift enrivonments.

I guess in this case - we will have to create a configmap with cinder predicate in openshift-kube-scheduler namespace by default via this operator if deployed on Openstack and operator will merge user specified configmap with our own configmap? Or we could install Cinder specific predicate in all environments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gnufied

  • The configmap needs to be created to in openshift-config namespace not in openshift-kube-scheduler namespace.
  • If you don't create in configmap in openshift-config namespace, we would start using Default provider similar to our old behaviour.(without scheduler-policy.json in older releases)
  • If you want to enable cinder specific predicate, you need to create a configmap with cinder predicate enabled in OpenShift config namespace. I will create a docs PR which talks about how to use the policy-configmap in 4.0 once I am back from vacation.

Copy link
Member

Choose a reason for hiding this comment

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

I understand how it works but I thought creating configmap in openshift-config was cluster-admin action? We want to ship Cinder predicate enabled by default in Openshift-4.0 installation on Openstack. This is not something admin should have to configure.

So lets say - if we did create the configmap policy-configmap in openshift-config namespace - so I guess admin will have to edit the configmap to make any changes to scheduler policy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd say creating configmap in openshift-config is the only way at this point of time. Usually it's done through UI as post-install operations. Let me check if there is a generic way for defining set of post-install operations in 4.0. If it exists, you can perhaps add the policy configmap there.

Choose a reason for hiding this comment

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

@ravisantoshgudimetla

If you don't create in configmap in openshift-config namespace, we would start using Default provider similar to our old behaviour.(without scheduler-policy.json in older releases)

What is the 'Default provider` bahavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants