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.

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

Added test reproducing a bug where consumable addon prevents from generating the next invoice #38

Merged
merged 4 commits into from May 14, 2019

Conversation

Projects
None yet
2 participants
@karser
Copy link

commented May 13, 2019

Hello @pierre ,
Please take a look at these 2 tests.
First one subscribes a user to a base monthly plan and checks that the invoice is created after 1 month.
Second subscribes a user to the same base plan and consumable addon and makes the same checks. For some reason, the addon prevents from creating the invoice. Recording usages doesn't make difference.

karser added some commits May 13, 2019

@karser

This comment has been minimized.

Copy link
Author

commented May 13, 2019

Tests passed, that's odd. The second test from this PR fails on my local KB.
image

@karser

This comment has been minimized.

Copy link
Author

commented May 13, 2019

I dropped my local database and tests now pass locally. But each test creates new tenant - they shouldn't interfere.
There is another KB instance deployed on a VPS with which tests fail against. if I drop the db, the tests will probably pass. Looks like a potential KB bug which prevents invoices generation.
Please tell me what do you think.

$this->assertCount(1, $invoices);
$this->assertSame(0.0, $invoices[0]->getAmount());
$this->clock->addDays(35);

This comment has been minimized.

Copy link
@pierre

pierre May 14, 2019

Member

After moving the clock, asynchronous processing of subscriptions and invoice generation will happen.

We have some logic to ensure all asynchronous jobs are fully processed before returning (provided the timeout is long enough), but the heuristic isn't always enough depending on the catalog, config, etc.

A good practice in these tests is to wait on the client side for an expected condition (e.g. a new invoice being generated) and fail the test if this doesn't happen after a timeout. See https://github.com/killbill/killbill-integration-tests/blob/2eec63809331bb0cb8217554542740352d5666aa/killbill-integration-tests/core/test_entitlement_create.rb#L29 for an example in our Ruby tests.

@karser

This comment has been minimized.

Copy link
Author

commented May 14, 2019

I ported wait_for_killbill and wait_for_expected_clause methods.
It reduced the tests time on CircleCI from 04:44 to 2:38. I guess that's the reason to merge this PR btw.

The tests still fail against my KB instance and invoices are not generated:
image

@pierre

This comment has been minimized.

Copy link
Member

commented May 14, 2019

The tests still fail against my KB instance and invoices are not generated:

Are you sure you don't have old data laying around? I suggest restarting your Kill Bill instance (to reset the clock) and let it fully process everything, in case there is a backlog of invoices to generate.

@pierre

pierre approved these changes May 14, 2019

@pierre pierre merged commit e4b05cc into killbill:work-for-release-0.20.x May 14, 2019

2 checks passed

ci/circleci: build-php-7.1 Your tests passed on CircleCI!
Details
ci/circleci: test-mysql-php-7.1 Your tests passed on CircleCI!
Details
@karser

This comment has been minimized.

Copy link
Author

commented May 14, 2019

I restarted the KB instance (down && up), but the issue still persists.

Are you sure you don't have old data laying around?

I do have old data under other tenants and apparently, that's why it is failing.

Sorry for being annoying, but if the same happens on production it's not that easy to drop the database and fix this issue. This might be a KB bug.

@pierre

This comment has been minimized.

Copy link
Member

commented May 14, 2019

I do have old data under other tenants and apparently, that's why it is failing.

Did you check the Kill Bill logs to understand why the invoice isn't generated?

Sorry for being annoying, but if the same happens on production it's not that easy to drop the database and fix this issue. This might be a KB bug.

Happy to investigate further if you think there is a bug, but I'm unable to reproduce the issue.

@karser

This comment has been minimized.

Copy link
Author

commented May 14, 2019

Here is how to reproduce this issue:

  1. Download and untar the mysql data directory:
curl https://s3.eu-west-3.amazonaws.com/killbill-test/mysql.tgz | tar xvz
  1. bind-mount this directory to /var/lib/mysql
    db:
        image: killbill/mariadb:0.20
        volumes:
            - "./mysql:/var/lib/mysql"
  1. docker compose down && docker compose up -d
  2. vendor/bin/phpunit -c phpunit.xml test/ServerInvoiceTest.php
@karser

This comment has been minimized.

Copy link
Author

commented May 14, 2019

And here is the log: https://s3.eu-west-3.amazonaws.com/killbill-test/killbill.out

I noticed an error in the log: Unknown column 'is_active' in 'field list'.

I tried running $MIGRATIONS_CMD but it didn't help. And there shouldn't be changes in DB structure because it has always been v0.20.10.

@pierre

This comment has been minimized.

Copy link
Member

commented May 14, 2019

It looks like you have an old version of killbill/mariadb:0.20. Could you delete your local image and volume, and try again?

@karser

This comment has been minimized.

Copy link
Author

commented May 14, 2019

Thanks, already did that.
My concern if this happens on production instance it wouldn't be so easy to fix it by dropping the db. Looks like the database went to the inconsistent state, although I've been staying all the time on v0.20.10.
Anyways, I hope this will never happen again, I'll be looking in the KB logs more closely.

@pierre

This comment has been minimized.

Copy link
Member

commented May 15, 2019

My concern if this happens on production instance it wouldn't be so easy to fix it by dropping the db. Looks like the database went to the inconsistent state, although I've been staying all the time on v0.20.10.

My guess is that your setup was incorrect from the beginning but it somehow didn't impact other tests.

Kill Bill is almost 10 years old now, and it has been many years since a critical bug was reported. Not saying there aren't any bug anymore, but we are highly confident the software is stable and production ready. It should not suddenly go to an inconsistent state. 😺

I'll be looking in the KB logs more closely.

This is good practice regardless, to make sure the system is behaving correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.