Skip to content

Add system-critical priority class to controller #229

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 1, 2019
Merged

Add system-critical priority class to controller #229

merged 1 commit into from
Mar 1, 2019

Conversation

ravisantoshgudimetla
Copy link
Contributor

Add system-cluster-critical priority class to controller, without this there is a chance that scheduler evicts these pods when there is a resource crunch.

@ravisantoshgudimetla
Add system-critical priority class to controller
19a4f55 
Add system-cluster-critical priority class to controller,  without this there is a chance that scheduler evicts these pods when there is a resource crunch.
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 27, 2019
@enxebre
Copy link
Member

enxebre commented Feb 27, 2019

/approve
needs #233

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enxebre

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:

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 27, 2019
@ingvagabund
Copy link
Member

/lgtm

/retest

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

/retest

Please review the full test history for this PR and help us cut down flakes.

6 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@ravisantoshgudimetla
Copy link
Contributor Author

@ingvagabund - Jan, I see that this PR failing tests continuously with error:

time="2019-02-28T09:40:27Z" level=info msg="Waiting for all clusterapi-manager-controllers deployment pods to be ready, have 0, expecting 1"

Any other changes that could have caused this issue?

@ingvagabund
Copy link
Member

@ravisantoshgudimetla lemme take a look

@ingvagabund
Copy link
Member

/retest

@ingvagabund
Copy link
Member

2 facts:

  • ci/jenkins/integration is over over pure k8s
  • controllers are deployed into openshift-machine-api namespace

At this point only kube-system can run critical pods. I will need to update the job to deploy into kube-system instead of openshift-machine-api.

@ravisantoshgudimetla
Copy link
Contributor Author

Thanks @ingvagabund.

At this point only kube-system can run critical pods.

That is right. Shouldn't we run CI over OpenShift 4.0 clusters?

@ingvagabund
Copy link
Member

ingvagabund commented Feb 28, 2019 •
edited
Loading

That is right. Shouldn't we run CI over OpenShift 4.0 clusters?

we do both, OpenShift 4.0 and Kubernetes.

I will fix the CI ASAP.

@ingvagabund
Copy link
Member

@ravisantoshgudimetla we are about to remove ci/jenkins/integration

@ingvagabund
Copy link
Member

ingvagabund commented Feb 28, 2019 •
edited
Loading

You don't need to do anything here, we will just remove the job and switch to a new one that deploys all controllers under kube-system. Just waiting until #220 lands. Then I can switch. Should not take more than 2hrs.

@ravisantoshgudimetla
Copy link
Contributor Author

@ingvagabund - Thank you :)

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

3 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@ravisantoshgudimetla
Copy link
Contributor Author

/hold

Till #220 lands. I will remove the hold once we are good to go.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2019
@ingvagabund
Copy link
Member

/refresh

@ingvagabund
Copy link
Member

/retest

@paulfantom
Copy link
Contributor

/override ci/jenkins/integration
/refresh

@openshift-ci-robot
Copy link
Contributor

@paulfantom: paulfantom unauthorized: /override is restricted to repo administrators

In response to this:

/override ci/jenkins/integration
/refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ingvagabund
Copy link
Member

/refresh

@ingvagabund
Copy link
Member

/retest

@paulfantom
Copy link
Contributor

/refresh

@ingvagabund
Copy link
Member

/retest

@ingvagabund
Copy link
Member

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 1, 2019
@openshift-merge-robot openshift-merge-robot merged commit c7adacf into openshift:master Mar 1, 2019
@enxebre enxebre mentioned this pull request Mar 14, 2019
Closed
ingvagabund pushed a commit to ingvagabund/machine-api-operator that referenced this pull request Jul 11, 2019
@openshift-merge-robot
Merge pull request openshift#229 from frobware/switch-build-to-go-1.12
342c4e2 
Switch build to go 1.12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enxebre enxebre Awaiting requested review from enxebre

@spangenberg spangenberg Awaiting requested review from spangenberg

Assignees

@ingvagabund ingvagabund

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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@ravisantoshgudimetla @enxebre @openshift-ci-robot @ingvagabund @openshift-bot @paulfantom @openshift-merge-robot