Skip to content

Add readiness and liveness probes #660

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

Merged
merged 2 commits into from
Nov 6, 2020

Conversation

skonto
Copy link
Collaborator

@skonto skonto commented Nov 2, 2020

  • Follows the approach here. Tested with a deployment. Uses the standard paths: /livez & /readyz, exposed by c-r.
  • We need this especially for integration tests to make sure everything is setup (monitoring is not covered due to the lack of this).
    /cc @markusthoemmes @aliok

@markusthoemmes
Copy link
Contributor

/test 4.6-upgrade-tests-aws-ocp-46

For once I'd like to see upgrade test passing.

@skonto
Copy link
Collaborator Author

skonto commented Nov 3, 2020

/test 4.6-upgrade-tests-aws-ocp-46

@skonto
Copy link
Collaborator Author

skonto commented Nov 3, 2020

@markusthoemmes all tests passed but it failed again.

@skonto
Copy link
Collaborator Author

skonto commented Nov 4, 2020

@markusthoemmes I think we can merge this one.

@markusthoemmes
Copy link
Contributor

I'd like the upgrade test to at least run once. Damn infra issues...

/test 4.6-upgrade-tests-aws-ocp-46

@skonto
Copy link
Collaborator Author

skonto commented Nov 5, 2020

Right now we are hitting the issue of the leader deadlock when using osdk: new pod passes Become if it becomes a leader but the old pod will not release that lock to allow that until the new pod becomes ready which will never happen (can happen after Become). Hopefully the c-r leader election mechanism will solve this, upgrade tests should pass if that is true.

@skonto
Copy link
Collaborator Author

skonto commented Nov 5, 2020

/retest

1 similar comment
@skonto
Copy link
Collaborator Author

skonto commented Nov 5, 2020

/retest

@skonto
Copy link
Collaborator Author

skonto commented Nov 6, 2020

21:20:04.620 INFO:    Wait for the index pod to be up to avoid inconsistencies with the catalog source.
21:20:04.622 DEBUG:   [[ $(oc get pods -l app=serverless-index -n openshift-marketplace -o name | wc -l) == '0' ]] : Waiting until non-zero (max 300 sec.)
done
21:20:15.764 DEBUG:   [[ $(oc get pods -l app=serverless-index -n openshift-marketplace -o 'jsonpath={..status.conditions[?(@.type=="Ready")].status}') != 'True' ]] : Waiting until non-zero (max 300 sec.)
.............................................................................................................................................................................................................................................................................................................
22:21:47.084 ERROR:   Time out of 300 exceeded

@skonto
Copy link
Collaborator Author

skonto commented Nov 6, 2020

/retest

@markusthoemmes
Copy link
Contributor

^^ Is a compile error in eventing, supposed to be fixed via openshift/knative-eventing#957. Sadly we need to wait for this as this hasn't yet tested upgradeability at this point.

@markusthoemmes
Copy link
Contributor

/retest

Compile error is hopefully gone.

Copy link
Contributor

@markusthoemmes markusthoemmes left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

Off you go!

@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markusthoemmes, skonto

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants