8000 Support for Varnish 6 and remove Varnish 2, 3 and 5 by ismaelpuerto · Pull Request #3360 · collectd/collectd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support for Varnish 6 and remove Varnish 2, 3 and 5 #3360

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
merged 9 commits into from
Mar 2, 2020
Merged

Support for Varnish 6 and remove Varnish 2, 3 and 5 #3360

merged 9 commits into from
Mar 2, 2020

Conversation

ismaelpuerto
Copy link
Contributor
@ismaelpuerto ismaelpuerto commented Dec 23, 2019 •
edited by dago
Loading

ChangeLog: varnish plugin: Support for Varnish 6, deprecated 2, 3 and 5.
ChangeLog: varnish plugin: Added counters for vmod-goto.

I keep Varnish 4 because Varnish Software keep support for Varnish Plus 4. Deleted Varnish 2, 3 and 5 because does't maintened: https://varnish-cache.org/releases/

@ismaelpuerto ismaelpuerto changed the title Support for Varnish 6 and remove Varnish 2 and 3 Support for Varnish 6 and remove Varnish 2, 3 and 5 Dec 26, 2019
@dago
Copy link
Contributor
dago commented Feb 29, 2020

@ismaelpuerto There definitely a failing CI test. Would you mind having a look?

@ismaelpuerto
Copy link
Contributor Author

Hello @dago, fail test of "trusty" because use old version of varnish, Ubuntu trusty born at 2014. Technically, trusty is a version not supported without subscription. https://wiki.ubuntu.com/Releases

@dago
Copy link
Contributor
dago commented Feb 29, 2020

@ismaelpuerto I see, I am a bit reluctant to throw out support for old versions when there is the possibility some it is still used in the field. However, I could imagine that people using varnish on platforms will probably run newer OS versions and people with older OSes probably do so for other datacenter non-web stuff.

@mrunge
Copy link
Member
mrunge commented Mar 2, 2020

I'd be good with dropping unsupported versions.

dago
dago approved these changes Mar 2, 2020
View reviewed changes
Copy link
Contributor
@dago dago left a comment

Choose a reason for hiding this comment

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

LGTM

@dago
Copy link
Contributor
dago commented Mar 2, 2020

@mrunge I am now merging this, but we need to take out building on "trusty" then. Can you tell me where this can be changed in the CI?

@dago dago merged commit afde89d into collectd:master Mar 2, 2020
@kkepka
Copy link
Contributor
kkepka commented Mar 2, 2020 •
edited
Loading

@dago if you would like to remove building on trusty at all, then quickshot would be on L38-40 in .cirrus.yml, but i can double check it on side if you want

@dago
Copy link
Contributor
dago commented Mar 2, 2020

@kkepka We can continue to build on trusty, but then Varnish needs to be disabled. Would you be willing to adjust the configuration?

@kkepka
Copy link
Contributor
kkepka commented Mar 2, 2020

ok, just --disable-varnish there is not enough, as it fails on /checks/check-built-plugins.sh, will look for right place to disable test for varnish on trusty

@dago dago mentioned this pull request Mar 3, 2020
Closed
@dago
Copy link
Contributor
dago commented Mar 3, 2020

@kkepka Can you please follow up on the CI issue with trusty? The broken CI makes it hard to see real build issues.

@dago dago mentioned this pull request Mar 4, 2020
Open
@kkepka
Copy link
Contributor
kkepka commented Mar 4, 2020

yes i can, getting into it right now

kkepka added a commit to collectd/ci-docker that referenced this pull request Mar 4, 2020
@kkepka
Remove support for varnish plugin on trusty
178dc31 
Remove varnish plugin on trusty from SUPPORTED_PLUGIN_LIST, because colectd 5.11 has dropped support for varnish 3 with collectd/collectd#3360
latest varnish version in trusty is 3.0.5 (https://launchpad.net/ubuntu/trusty/+package/varnish) and there won't be any upgrades of it, as it's not supported anymore (https://wiki.ubuntu.com/Releases)
@kkepka kkepka mentioned this pull request Mar 4, 2020
Merged
@kkepka
Copy link
Contributor
kkepka commented Mar 4, 2020

short backtrace of reason for collectd/ci-docker#3:

  • ci fails with missing varnish plugin on trusty
  • failing script check-built-plugins.sh uses SUPPORTED_PLUGIN_LIST env var to check if supported plugins have been built
  • content of this var is set in Dockerfile during ci container image build by travis on per target branch (trusty_amd64) image/distro

@dago dago mentioned this pull request Mar 11, 2020
Closed
@rubenk
Copy link
Contributor
rubenk commented Mar 13, 2020

Can we revert this for now, at least until we managed to merge 5.10 and 5.11 back into master and released 5.11?

@mrunge
Copy link
Member
mrunge commented Mar 13, 2020

@rubenk would you open a PR to revert or shall I?

@rubenk
Copy link
Contributor
rubenk commented Mar 13, 2020

@mrunge please do

@mrunge
Copy link
Member
mrunge commented Mar 13, 2020

Submitted #3412

@rubenk
Copy link
Contributor
rubenk commented Mar 13, 2020

Ubuntu 18.04 still has varnish 5 and is widely used (also for Travis). Can we keep support for Varnish 5 a little longer?

@rubenk rubenk mentioned this pull request Mar 13, 2020
Closed
@mrunge
Copy link
Member
mrunge commented Mar 13, 2020

#3414 has all reverts minus the noop commit 039ccb9 from @dago

Cactusbone added a commit to F4-Group/collectd that referenced this pull request Apr 23, 2020
@Cactusbone
Add varnish 6 new values
0df7299 
Extracted from collectd#3360
@Cactusbone Cactusbone mentioned this pull request Apr 27, 2020
Merged
Cactusbone added a commit to F4-Group/collectd that referenced this pull request Apr 27, 2020
@Cactusbone
Add varnish 6 new values
bc32818 
Extracted from collectd#3360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dago dago dago approved these changes

@mrunge mrunge Awaiting requested review from mrunge

@kwiatrox kwiatrox Awaiting requested review from kwiatrox

@rpv-tomsk rpv-tomsk Awaiting requested review from rpv-tomsk

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
@ismaelpuerto @dago @mrunge @kkepka @rubenk
0