kubernetes-sigs / prow

Prow is a Kubernetes based CI/CD system developed to serve the Kubernetes community. This repository contains Prow source code and Hugo sources for Prow documentation site.
https://docs.prow.k8s.io
Apache License 2.0
129 stars 99 forks source link

`deck`: move readiness serve to where we actually start the server #317

Closed smg247 closed 1 week ago

smg247 commented 2 weeks ago

This is a follow up to the failed attempt from https://github.com/kubernetes-sigs/prow/pull/308. It makes it more clear that the CSRF logic wraps the server, and starts the readiness endpoint immediately before starting the server.

netlify[bot] commented 2 weeks ago

Deploy Preview for k8s-prow ready!

Name Link
Latest commit 2a618c8f17e6c6e640d95799df80a861eac05c59
Latest deploy log https://app.netlify.com/sites/k8s-prow/deploys/672a7217619cba000820ec39
Deploy Preview https://deploy-preview-317--k8s-prow.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

smg247 commented 2 weeks ago

/cc @petr-muller

k8s-ci-robot commented 1 week ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: petr-muller, smg247

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: - ~~[cmd/deck/OWNERS](https://github.com/kubernetes-sigs/prow/blob/main/cmd/deck/OWNERS)~~ [petr-muller,smg247] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
petr-muller commented 1 week ago

/hold

Can we remove the --allow-insecure from the tests again? Or at least from one, to integeration-test both paths?

smg247 commented 1 week ago

Can we remove the --allow-insecure from the tests again? Or at least from one, to integeration-test both paths?

Since the other PR was reverted, this has required no changes to the integration tests at all. It is not present in the deck_tenant_deployment now.

smg247 commented 1 week ago

/hold cancel

petr-muller commented 1 week ago

Since the other PR was reverted, this has required no changes to the integration tests at all. It is not present in the deck_tenant_deployment now.

Ugh, I forgot that because we reverted the test manifests are now in the state I wanted. Code reviews on Friday :grimacing: