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.

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

adjust jenkins template setting to account for effects of constrained… #18832

Merged
merged 1 commit into from
Mar 8, 2018
Merged

adjust jenkins template setting to account for effects of constrained… #18832

merged 1 commit into from
Mar 8, 2018

Conversation

gabemontero
Copy link
Contributor

… default max heap

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1551500
Fixes openshift/jenkins#501

@openshift/sig-developer-experience ptal

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 5, 2018
bparees
bparees requested changes Mar 5, 2018
View reviewed changes
examples/jenkins/jenkins-ephemeral-template.json Outdated
@@ -26,7 +26,8 @@
"metadata": {
"name": "${JENKINS_SERVICE_NAME}",
"annotations": {
"template.openshift.io/expose-uri": "http://{.spec.host}{.spec.path}"
"template.openshift.io/expose-uri": "http://{.spec.host}{.spec.path}",
"haproxy.router.openshift.io/timeout": "2m"
Copy link
Contributor

Choose a reason for hiding this comment

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

alignment

examples/jenkins/jenkins-ephemeral-template.json Outdated
"initialDelaySeconds": 3,
"httpGet": {
"path": "/login",
"port": 8080
}
},
"livenessProbe": {
"timeoutSeconds": 3,
"timeoutSeconds": 240,
"periodSeconds": 360,
Copy link
Contributor

Choose a reason for hiding this comment

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

alignment

examples/jenkins/jenkins-persistent-template.json Outdated
@@ -26,7 +26,8 @@
"metadata": {
"name": "${JENKINS_SERVICE_NAME}",
"annotations": {
"template.openshift.io/expose-uri": "http://{.spec.host}{.spec.path}"
"template.openshift.io/expose-uri": "http://{.spec.host}{.spec.path}",
"haproxy.router.openshift.io/timeout": "2m"
Copy link
Contributor

Choose a reason for hiding this comment

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

alignment

examples/jenkins/jenkins-persistent-template.json Outdated
"initialDelaySeconds": 3,
"httpGet": {
"path": "/login",
"port": 8080
}
},
"livenessProbe": {
"timeoutSeconds": 3,
"timeoutSeconds": 240,
"periodSeconds": 360,
Copy link
Contributor

Choose a reason for hiding this comment

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

alignment

@bparees
Copy link
Contributor

bparees commented Mar 5, 2018

@sdodson fyi, we're going to need to pull this change into openshift-ansible once it merges.

@gabemontero please make this change in here too: https://github.com/openshift/jenkins/tree/master/openshift/templates (once my work finishes, you'll only need to make the change in the jenkins repo and it will filter down from jenkins->library->origin, but we're in an in between state right now).

@gabemontero
Copy link
Contributor Author

10-4 @bparees

@gabemontero
Copy link
Contributor Author

There is also a lot of history and diagnostic details in openshift/jenkins#440

@gabemontero
Copy link
Contributor Author

also @sdodson this is the master branch PR

given the current state of master, I created #18834 for the 3.9 branch

bparees
bparees reviewed Mar 5, 2018
View reviewed changes
examples/jenkins/jenkins-ephemeral-template.json Outdated
"initialDelaySeconds": 3,
"httpGet": {
"path": "/login",
"port": 8080
}
},
"livenessProbe": {
"timeoutSeconds": 3,
"timeoutSeconds": 240,
"periodSeconds": 360,
"initialDelaySeconds": 420,
"failureThreshold" : 30,
Copy link
Contributor

Choose a reason for hiding this comment

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

with a polling interval of 6 mins, i think the failure threshold (number of failures required before we consider it dead) needs to be reduced.

@gabemontero gabemontero force-pushed the jenkins-template-adjust branch from 767b0c5 to 66bc0cb Compare March 5, 2018 17:55
@gabemontero
Copy link
Contributor Author

updates pushed @bparees

@gabemontero
Copy link
Contributor Author

fyi closed out 3.9 #18834

will create a new 3.9 PR off of this commit once this one gets approved

@bparees
Copy link
Contributor

bparees commented Mar 5, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 5, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, gabemontero

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 Mar 5, 2018
@openshift-bot
Copy link
Contributor

/retest

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

@gabemontero
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

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

@gabemontero
Copy link
Contributor Author

/retest

@gabemontero gabemontero mentioned this pull request Mar 7, 2018
Merged
@bparees
Copy link
Contributor

bparees commented Mar 7, 2018

/retest

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 18780, 18802, 18391, 18832, 18808).

@openshift-merge-robot openshift-merge-robot merged commit 24951be into openshift:master Mar 8, 2018
@gabemontero gabemontero deleted the jenkins-template-adjust branch March 8, 2018 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bparees bparees bparees requested changes

@jupierce jupierce Awaiting requested review from jupierce

Assignees

@bparees bparees

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. sig/developer-experience size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@gabemontero @bparees @openshift-ci-robot @openshift-bot @openshift-merge-robot