magento2
magento2 copied to clipboard
[vcl] Better debug
Detecting if the response is a hit usingthe x-varnish
header is flaky
and notably doesn't work in a multi-tier setup.
On top of this, HIT/MISS is a false dichotomy (https://info.varnish-software.com/blog/using-obj-hits) and while I'd like something more detailed like https://docs.varnish-software.com/tutorials/hit-miss-logging/ (tech version of the blog), I recognize that it's "a lot" of VCL, so this commit only differentiates between hits, misses and passes, the latter being pretty useful to identify while debugging.
note: all versions have been changed for the sake of consistency but both the 4.x and 5.x series have been EOL'd a (long) while ago and users should be encouraged to upgraded as soon as possible.
edit: I've added a test that you can run with varnishtest dev/tests/varnish/*.vtc
since the current php
suite doesn't correctly test the committed VCL. It would probably be smart to run that command as part of the test suite, but I'll leave that to more knowledgeable people than myself.
Contribution checklist (*)
- [ ] Pull request has a meaningful description of its purpose
- [ ] All commits are accompanied by meaningful commit messages
- [ ] All new or changed code is covered with unit/integration tests (if applicable)
- [ ] All automated tests passed successfully (all builds are green)
Resolved issues:
- [x] resolves magento/magento2#37912: [vcl] Better debug
Hi @gquintard. Thank you for your contribution Here is some useful tips how you can test your changes using Magento test environment. Add the comment under your pull request to deploy test or vanilla Magento instance:
-
@magento give me test instance
- deploy test instance based on PR changes -
@magento give me 2.4-develop instance
- deploy vanilla Magento instance
:exclamation: Automated tests can be triggered manually with an appropriate comment:
-
@magento run all tests
- run or re-run all required tests against the PR changes -
@magento run <test-build(s)>
- run or re-run specific test build(s) For example:@magento run Unit Tests
<test-build(s)>
is a comma-separated list of build names. Allowed build names are:
-
Database Compare
-
Functional Tests CE
-
Functional Tests EE
, -
Functional Tests B2B
-
Integration Tests
-
Magento Health Index
-
Sample Data Tests CE
-
Sample Data Tests EE
-
Sample Data Tests B2B
-
Static Tests
-
Unit Tests
-
WebAPI Tests
You can find more information about the builds here
:information_source: Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.
For more details, please, review the Magento Contributor Guide documentation.
@magento run all tests
@magento run Unit Tests
Hi @lbajsarowicz, thank you for the review. ENGCOM-7766 has been created to process this Pull Request :eight_spoked_asterisk: @lbajsarowicz, could you please add one of the following labels to the Pull Request?
Label | Description |
---|---|
Auto-Tests: Covered |
All changes in Pull Request is covered by auto-tests |
Auto-Tests: Not Covered |
Changes in Pull Request requires coverage by auto-tests |
Auto-Tests: Not Required |
Changes in Pull Request does not require coverage by auto-tests |
@magento run Integration Tests, Functional Tests CE
@lbajsarowicz, apologies for my ignorance, is there anything I should do here?
@gquintard No. Now we wait for the process of the merge to be performed.
Automated Tests are not applicable, as the change is related to infrastructure configuration.
Dev experience is required for testing of this PR. Please note that Manual testing has not been performed.
@engcom-Bravo seems like you missed to assign the PR. I just did it for you
@engcom-Charlie basicwlly varnish 4 already reached end of life https://info.varnish-software.com/blog/options-at-varnish-cache-4-end-of-support . I believe if everything works fine on 5 and 6 - let’s revert changes in varnish4 config
Indeed, obj.uncacheable
was introduced later, I missed that, sorry.
@ihor-sviziev , 5.X is also EOL, if you want to just focus on 6.X: https://varnish-cache.org/releases/index.html
@gquintard could you just revert changes for varnish4 and varnish5 than?
done
@magento run all tests
Hi @ihor-sviziev, thank you for the review. ENGCOM-7766 has been created to process this Pull Request
@magento run Functional Tests CE, Functional Tests EE
functional tests EE are failing, but I don't see anyway for them to fail the way they do. False negative?
@magento run Functional Tests EE
@magento run Functional Tests EE
bump?
hi, we are almost at the one-year anniversary of that PR, is there something I can do to make it happen?
@gquintard, unfortunately, this PR has low priority and will be processed after all high priority ones
@gquintard can you resolve conflicts
@mrtuvn , done
@magento run all tests. Rerun checks
Failed to run the builds. Please try to re-run them later.
@magento create issue
@magento run all tests
The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.