Skip to content

Update ifrit for gorouter #35

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
markstgodard merged 1 commit into from
Jul 7, 2016
Merged

Update ifrit for gorouter #35

markstgodard merged 1 commit into from
Jul 7, 2016

Conversation

@saliceti
Copy link
Contributor

@saliceti saliceti commented Jun 28, 2016

What

gorouter relies on ifrit to manage its processes. But the version pakaged in routing-release doesn't allow gorouter to receive more than one signal.
This breaks the drain script as it repeatedly sends signal USR1 while gorouter is draining the connections, until gorouter finally stops. Without the fix, gorouter dies after 5 seconds only when the drain script sends the second signal.
This commit in ifrit allows to receive more than one signal. This PR updates the ifrit submodule to include this change.

How to test

  • Start gorouter with drain_wait: 60 for example
  • Run the drain script once
  • Run the drain script after 5 seconds (same as bosh-agent does)
  • In current version, gorouter dies. With the fix, it continues draining the connections and stops after 60s

@saliceti
Update ifrit for gorouter
ce98370 
The ifrit version doesn't allow gorouter to receive more than one
signal. This breaks the drain script as it repeatedly sends signal USR1
while gorouter is draining the connections, until gorouter finally
stops. Without the fix, gorouter dies after 5 seconds only when the
drain script sends the second signal.
@cfdreddbot
Copy link

cfdreddbot commented Jun 28, 2016

Hey saliceti!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot
Copy link

cf-gitbot commented Jun 28, 2016

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/123068391

The labels on this github issue will be updated when the story is started.

combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 28, 2016
Separate routing-release from cf-releases
21e2d79 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 28, 2016
Separate routing-release from cf-releases
5ddda6a 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 29, 2016
Separate routing-release from cf-releases
cffb898 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 29, 2016
Separate routing-release from cf-releases
758e0b5 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
@cf-gitbot cf-gitbot added scheduled We agree this change makes sense and plan to work on it ourselves at some point. and removed unscheduled labels Jun 29, 2016
@dcarley
Copy link

dcarley commented Jun 30, 2016

We discovered this problem when implementing availability tests in our CF deployment pipeline. They make a requests every 100ms to an app that is deployed on CF whilst a bosh deploy of CF is happening. We were seeing several requests dropped, which coincided with the router jobs updating. With the change in this PR no requests were dropped and we were able to raise the threshold of our test to 100%.

There's some more detail about the test here if you're interested: govuk-paas/paas-cf#321

combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 30, 2016
Separate routing-release from cf-releases
4f81ab3 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 30, 2016
Separate routing-release from cf-releases
1880476 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
saliceti pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 30, 2016
@saliceti
Separate routing-release from cf-releases
7fd7ff0 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
saliceti pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 30, 2016
@saliceti
Separate routing-release from cf-releases
1322e34 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
keymon pushed a commit to govuk-paas/paas-cf that referenced this pull request Jun 30, 2016
@keymon
Separate routing-release from cf-releases
68ddc7f 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

More about bug can be found here:
cloudfoundry/routing-release#35
keymon pushed a commit to govuk-paas/paas-cf that referenced this pull request Jul 1, 2016
@keymon
Separate routing-release from cf-releases
f8bbfef 
Due to a bug in gorouter we have to split routing release from main
cf-release and point router job to the newly created fork.

We upload and deploy the our forked routing-release as a development release.

More about bug can be found here:
cloudfoundry/routing-release#35
@cf-gitbot cf-gitbot added in progress and removed scheduled We agree this change makes sense and plan to work on it ourselves at some point. labels Jul 6, 2016
@markstgodard markstgodard merged commit 6c46745 into cloudfoundry:develop Jul 7, 2016
@markstgodard
Copy link
Contributor

markstgodard commented Jul 7, 2016

Thanks for the PR!

We tested and merged. thanks!

@markstgodard && @shashwathi

@crhino crhino mentioned this pull request Jul 9, 2016
Closed
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Sep 1, 2016
Revert "Separate routing-release from cf-releases"
6627494 
This reverts commit f8bbfef.

As ifrit fix (
    cloudfoundry/routing-release#35)
is already included in routing release used by cf-release we can remove
our fork.
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Sep 2, 2016
Revert "Separate routing-release from cf-releases"
9892a1b 
This reverts commit f8bbfef.

As ifrit fix (
    cloudfoundry/routing-release#35)
is already included in routing release used by cf-release we can remove
our fork.
keymon pushed a commit to govuk-paas/paas-cf that referenced this pull request Sep 2, 2016
@keymon
Revert "Separate routing-release from cf-releases"
0d78c78 
This reverts commit f8bbfef.

As ifrit fix (
    cloudfoundry/routing-release#35)
is already included in routing release used by cf-release we can remove
our fork.
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Sep 2, 2016
Revert "Separate routing-release from cf-releases"
710ed88 
This reverts commit f8bbfef.

As ifrit fix (
    cloudfoundry/routing-release#35)
is already included in routing release used by cf-release we can remove
our fork.
combor pushed a commit to govuk-paas/paas-cf that referenced this pull request Sep 2, 2016
Revert "Separate routing-release from cf-releases"
0247544 
This reverts commit f8bbfef.

As ifrit fix (
    cloudfoundry/routing-release#35)
is already included in routing release used by cf-release we can remove
our fork.
winkingturtle-vmw pushed a commit that referenced this pull request Sep 11, 2023
bump cf-tcp-router
8936574 
bump gorouter
bump multierror
bump route-registrar
bump routing-api
bump routing-api-cli
bump routing-info

Submodule src/code.cloudfoundry.org/cf-tcp-router 004af19..89625e5:
  > Use bin/test.bash for running tests (#15)
Submodule src/code.cloudfoundry.org/gorouter 31a88e6..587fdbd:
  > Use bin/test.bash for running tests (#357)
Submodule src/code.cloudfoundry.org/multierror 0623381..add4c8e:
  > Merge pull request #6 from cloudfoundry/with-bin/test.bash
Submodule src/code.cloudfoundry.org/route-registrar 247293e..958da78:
  > Use bin/test.bash for running tests (#35)
Submodule src/code.cloudfoundry.org/routing-api dd977026..84d22269:
  > Use bin/test.bash for running tests (#35)
Submodule src/code.cloudfoundry.org/routing-api-cli 045f777..5c9811f:
  > Use bin/test.bash for running tests (#16)
Submodule src/code.cloudfoundry.org/routing-info 079a2734..3a6d4ccb:
  > Use bin/test.bash for running tests (#5)
@winkingturtle-vmw winkingturtle-vmw mentioned this pull request Sep 11, 2023
Merged
9 tasks
winkingturtle-vmw added a commit that referenced this pull request Sep 11, 2023
@winkingturtle-vmw
bump cf-tcp-router (#343)
62c613d 
bump gorouter
bump multierror
bump route-registrar
bump routing-api
bump routing-api-cli
bump routing-info

Submodule src/code.cloudfoundry.org/cf-tcp-router 004af19..89625e5:
  > Use bin/test.bash for running tests (#15)
Submodule src/code.cloudfoundry.org/gorouter 31a88e6..587fdbd:
  > Use bin/test.bash for running tests (#357)
Submodule src/code.cloudfoundry.org/multierror 0623381..add4c8e:
  > Merge pull request #6 from cloudfoundry/with-bin/test.bash
Submodule src/code.cloudfoundry.org/route-registrar 247293e..958da78:
  > Use bin/test.bash for running tests (#35)
Submodule src/code.cloudfoundry.org/routing-api dd977026..84d22269:
  > Use bin/test.bash for running tests (#35)
Submodule src/code.cloudfoundry.org/routing-api-cli 045f777..5c9811f:
  > Use bin/test.bash for running tests (#16)
Submodule src/code.cloudfoundry.org/routing-info 079a2734..3a6d4ccb:
  > Use bin/test.bash for running tests (#5)
Dariquest pushed a commit to sap-contributions/routing-release that referenced this pull request May 5, 2025
@winkingturtle-vmw @ebroberson
Use bin/test.bash for running tests (cloudfoundry#35)
958da78 
- Use natsServer built within the pipeline instead of finding one on
  path
- Remove RELEASE_DIR since it's no longer needed at routing-release
  level

Signed-off-by: Amin Jamali <ajamali@vmware.com>
Signed-off-by: Brandon Roberson <broberson@vmware.com>
Co-authored-by: Brandon Roberson <broberson@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@saliceti @cfdreddbot @cf-gitbot @dcarley @markstgodard