Traefik: 1.7.14 -> 2.2.0 #76723

jokogr · 2019-12-31T01:00:53Z

Motivation for this change

I was experimenting with Traefik and thought to update the package to the latest v2 one.

In the mean time, buildGoPackage was switched to buildGoModule and the respective NixOS module has been adapted to the new configuration requirements (static and dynamic, routers, middlewares, services and so on).

Finally, I have removed hamhut1066 from the maintainers list, as this user is no longer in GitHub.

Fixes #76518.

Things done

Notify maintainers

cc @vdemeester

Br1ght0ne

diff for package LGTM
didn't check module
executable shows usage

[5 built, 5 copied (432.5 MiB), 104.1 MiB DL]
https://github.com/NixOS/nixpkgs/pull/76723
1 package built:
traefik

dali99 · 2020-01-05T22:37:13Z

Considering that the configuration isn't backwards compatible, should this maybe go into a separate package/module?

traefik 1.7 is still supported upstream until ~~september 2020~~ 2021, and this kind of change should definetly not be backported to 19.09

having this be the only option on master and 20.03 seems reasonable

Otherwise LGTM!

EDIT: https://containo.us/blog/traefik-2-1-in-the-wild/

jokogr · 2020-01-07T10:27:39Z

Considering that the configuration isn't backwards compatible, should this maybe go into a separate package/module?

traefik 1.7 is still supported upstream until september 2020, and this kind of change should definetly not be backported to 19.09

having this be the only option on master and 20.03 seems reasonable

Otherwise LGTM!

Thanks for having a look on this PR. I do not understand your initial question, as in whether to have a separate package / module. As you point out after the question and I agree with this, we could maintain Traefik v1.7 on 19.09 only and v2.1 and later on 20.03 and master. Why should we have additional maintenance effort on master?

@vdemeester what's your view on this?

dali99 · 2020-01-07T12:42:33Z

Yeah I'm just not sure on what the general policy is on changes like this. It's kind of two entirely different pieces of software, and 20.09 is the first release we can do where 1.7 is no longer supported upstream.

I'm personally okay with 2.1 being the only thing available on master and 20.03, but I just wanted to put the info here so other's could chime in.

dali99 · 2020-01-20T16:47:06Z

@vdemeester
@kalbasit

Friendly pings :D (3 weeks) has this been forgotten?

nixos-discourse · 2020-02-02T14:27:01Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixos-20-03-feature-freeze/5655/17

pkgs/servers/traefik/default.nix

kalbasit · 2020-02-02T21:21:40Z

nixos/modules/services/web-servers/traefik.nix

      '';
      type = types.attrs;
      default = {
-        defaultEntryPoints = ["http"];
        entryPoints.http.address = ":80";


Don't you need to set entryPoints.web.address like in the example case?

To be honest, I didn't see anything defaultEntryPoints on the official documentation site, so I skipped this.

This issue on the official traefik GitHub repo supports this.

jokogr · 2020-02-08T11:49:52Z

Thanks @kalbasit for the review and sorry for the wait! I have made the following changes:

Refactored to current master.
Updated to Traefik v2.1.4.
Removed goPackagePath as suggested.
Made a new commit in the relevant NixOS service to make configuration deep mergeable as in 00eb055162a52a2d9f42ba6c0eeec892c83bfa58 (thanks @Mic92 for the hint)

jokogr · 2020-03-02T18:56:01Z

Thanks @rnhmjoj for the review! I have made the following changes:

Rebased to current master.
Updated traefik to v2.1.6.
Switched to makeFlagsArray as suggested by @rnhmjoj (sorry, rebasing took away @kalbasit's and @rnhmjoj's comments).

rnhmjoj · 2020-03-03T18:28:59Z

So, the changes look good and I tested the package update.
I would be more confident to merge this if someone would review the module: it's a substantial change. Also it would be good to have a NixOS test, even a simple one.

nixos-discourse · 2020-03-15T20:20:58Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/130

nixos/modules/services/web-servers/traefik.nix

pkgs/servers/traefik/default.nix

dali99 · 2020-03-22T00:45:12Z

Changed the module to use attrsOf instead of lazyAttrsOf as a test on 19.09

hash mismatch in fixed-output derivation '/nix/store/hb3xrk6vzszz87vz6z0sk1zdcjssf0r3-traefik-2.1.6-go-modules':
  wanted: sha256:1mdzjc9v54b0gc8f97gpdgym1dxhyc1p1qivgfvjraj3vgfc7i5g                          
  got:    sha256:1chd174wplr0sqyij4z05kpnhs81sd3xc7jjlb2jlxmbnsf9kay5

changing the hash:

build github.com/containous/traefik/v2/internal: cannot load crypto/ed25519: cannot find module providing package crypto/ed25519
generate.go:5: running "go": exit status 1

~~I dont actually know the difference between lazyAttrsOf and attrsOf semantically, so that could be the reason its failing like it is, in that case disregard this comment.~~

EDIT: Package should build anyways, but it doesnt

20.03 should be out soon, Ill try build it ontop of that if (it doesnt get merged into it) when that happens.

jokogr · 2020-04-04T17:31:13Z

Rebased to current master.
Updated traefik to v2.2.0.
Switched from buildInputs to nativeBuildInputs as suggested by @Mic92.

@dali99 please, do not use words like Package should build anyways, but it doesnt, it gives a wrong impression of this PR.

Traefik as of now requires Go 1.14 (which is the default version currently in unstable) and NixOS 19.09 merely supports 1.13 and it is not working as in unstable, as you have noticed.

It happens to use this package (and NixOS module) on my servers running NixOS 19.09 and it works as intended after adding Go 1.14 in a package overlay:

  buildGo114Module = super.callPackage ./go-modules_generic.nix {
    go = self.buildPackages.go_1_14;
  };
  go_1_14 = super.callPackage ./go_1.14 {
    inherit (super.darwin.apple_sdk.frameworks) Security Foundation;
  };
  traefik = super.callPackage ./traefik {
    buildGoModule = self.buildGo114Module;
  };

The versions and the files I have copied there is left to the readers as an exercise. As I have mentioned before, I would like to focus my efforts on unstable to speed up the update process.

@rnhmjoj I will try to create a NixOS test for traefik today or tomorrow.

dali99 · 2020-04-04T23:31:05Z

I'm sorry I was just trying to help 😞

Good that I was just personally failing to build it though!

jokogr · 2020-04-06T06:44:42Z

I'm sorry I was just trying to help disappointed

Good that I was just personally failing to build it though!

No worries, I know you meant well, but I also do not want reviewers to have an excuse for not reviewing this 😃

Added a test to check the Docker and file providers.

pkgs/servers/traefik/default.nix

jokogr · 2020-04-06T15:55:54Z

Added passthru.tests as suggested by @Mic92.

jokogr · 2020-04-06T17:12:57Z

Added nixosTests in the argument list. 🙄

jokogr · 2020-04-06T18:30:22Z

Set a limit on x86_64-linux for the test, as it relies on docker-containers, whose tests have the same limit.

nixos/modules/services/web-servers/traefik.nix

This commit: 1. Updates the path of the traefik package, so that the out output is used. 2. Adapts the configuration settings and options to Traefik v2. 3. Formats the NixOS traefik service using nixfmt.

jokogr · 2020-04-12T20:54:29Z

Rebased to current master.
Dropped recursiveMerge as requested by @rnhmjoj.
Formatted the package and the service nix files using nixfmt to address a code change request by @rnhmjoj.

rnhmjoj

Well, I don't have anything else to add: it looks pretty good to me.
I double checked the package and the moduble, everything builds and ofBorgs is happy.
Thank you for adding a test: it will make future changes easier to review.

I'll wait a bit for the other reviewers if the have anything else to say, before merging.

ofborg bot added 6.topic: nixos 8.has: module (update) labels Dec 31, 2019

ofborg bot requested review from kalbasit and vdemeester December 31, 2019 01:33

ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Dec 31, 2019

Br1ght0ne approved these changes Dec 31, 2019

View reviewed changes

lo1tuma mentioned this pull request Jan 22, 2020

aws-sam-cli: 0.34.0 -> 0.40.0 #78281

Merged

10 tasks

kalbasit reviewed Feb 2, 2020

View reviewed changes

jokogr force-pushed the u/traefik-2.1.1 branch 2 times, most recently from 4b9b99d to 9b7d1a1 Compare February 8, 2020 11:41

ofborg bot requested a review from kalbasit February 8, 2020 12:08

jokogr changed the title ~~Traefik: 1.7.14 -> 2.1.1~~ Traefik: 1.7.14 -> 2.1.4 Feb 8, 2020

ngerstle mentioned this pull request Feb 22, 2020

traefik: 1.7.14 -> 1.7.21 #80824

Closed

10 tasks

jokogr force-pushed the u/traefik-2.1.1 branch from 0ffe191 to ad32fb5 Compare March 2, 2020 18:51

jokogr changed the title ~~Traefik: 1.7.14 -> 2.1.4~~ Traefik: 1.7.14 -> 2.1.6 Mar 2, 2020

Mic92 reviewed Mar 16, 2020

View reviewed changes

nixos/modules/services/web-servers/traefik.nix Show resolved Hide resolved

Mic92 reviewed Mar 18, 2020

View reviewed changes

pkgs/servers/traefik/default.nix Outdated Show resolved Hide resolved

jokogr force-pushed the u/traefik-2.1.1 branch from ad32fb5 to 211b7fe Compare April 4, 2020 17:02

ofborg bot added the 10.rebuild-darwin: 1 label Apr 4, 2020

jokogr changed the title ~~Traefik: 1.7.14 -> 2.1.6~~ Traefik: 1.7.14 -> 2.2.0 Apr 4, 2020

Mic92 reviewed Apr 6, 2020

View reviewed changes

pkgs/servers/traefik/default.nix Show resolved Hide resolved

jokogr force-pushed the u/traefik-2.1.1 branch from 14f7d7a to bbd3e7f Compare April 6, 2020 15:55

jokogr force-pushed the u/traefik-2.1.1 branch from bbd3e7f to 5f3e2a9 Compare April 6, 2020 17:06

jokogr force-pushed the u/traefik-2.1.1 branch from 5f3e2a9 to ebb40aa Compare April 6, 2020 18:28

rnhmjoj requested changes Apr 7, 2020

View reviewed changes

nixos/modules/services/web-servers/traefik.nix Outdated Show resolved Hide resolved

nixos/modules/services/web-servers/traefik.nix Outdated Show resolved Hide resolved

jokogr added 4 commits April 12, 2020 22:50

traefik: 1.7.14 -> 2.2.0

07f1844

nixos/traefik: Adapt to traefik v2

bc766b0

This commit: 1. Updates the path of the traefik package, so that the out output is used. 2. Adapts the configuration settings and options to Traefik v2. 3. Formats the NixOS traefik service using nixfmt.

nixos/traefik: make config deep mergeable

1f61fbf

nixos/traefik: add test

9360e37

jokogr force-pushed the u/traefik-2.1.1 branch from ebb40aa to 9360e37 Compare April 12, 2020 20:51

rnhmjoj approved these changes Apr 13, 2020

View reviewed changes

Mic92 merged commit 4c3f1d3 into NixOS:master Apr 13, 2020

jokogr deleted the u/traefik-2.1.1 branch April 13, 2020 16:52

flokli mentioned this pull request Jun 1, 2020

traefik: ship webui #41378

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Traefik: 1.7.14 -> 2.2.0 #76723

Traefik: 1.7.14 -> 2.2.0 #76723

jokogr commented Dec 31, 2019

Br1ght0ne left a comment

dali99 commented Jan 5, 2020 •

edited

jokogr commented Jan 7, 2020

dali99 commented Jan 7, 2020

dali99 commented Jan 20, 2020

nixos-discourse commented Feb 2, 2020

kalbasit Feb 2, 2020

jokogr Feb 8, 2020

jokogr commented Feb 8, 2020

jokogr commented Mar 2, 2020

rnhmjoj commented Mar 3, 2020

nixos-discourse commented Mar 15, 2020

dali99 commented Mar 22, 2020 •

edited

jokogr commented Apr 4, 2020

dali99 commented Apr 4, 2020 •

edited

jokogr commented Apr 6, 2020

jokogr commented Apr 6, 2020 •

edited

jokogr commented Apr 6, 2020

jokogr commented Apr 6, 2020

jokogr commented Apr 12, 2020

rnhmjoj left a comment

Traefik: 1.7.14 -> 2.2.0 #76723

Traefik: 1.7.14 -> 2.2.0 #76723

Conversation

jokogr commented Dec 31, 2019

Motivation for this change

Things done

Notify maintainers

Br1ght0ne left a comment

Choose a reason for hiding this comment

dali99 commented Jan 5, 2020 • edited

jokogr commented Jan 7, 2020

dali99 commented Jan 7, 2020

dali99 commented Jan 20, 2020

nixos-discourse commented Feb 2, 2020

kalbasit Feb 2, 2020

Choose a reason for hiding this comment

jokogr Feb 8, 2020

Choose a reason for hiding this comment

jokogr commented Feb 8, 2020

jokogr commented Mar 2, 2020

rnhmjoj commented Mar 3, 2020

nixos-discourse commented Mar 15, 2020

dali99 commented Mar 22, 2020 • edited

jokogr commented Apr 4, 2020

dali99 commented Apr 4, 2020 • edited

jokogr commented Apr 6, 2020

jokogr commented Apr 6, 2020 • edited

jokogr commented Apr 6, 2020

jokogr commented Apr 6, 2020

jokogr commented Apr 12, 2020

rnhmjoj left a comment

Choose a reason for hiding this comment

dali99 commented Jan 5, 2020 •

edited

dali99 commented Mar 22, 2020 •

edited

dali99 commented Apr 4, 2020 •

edited

jokogr commented Apr 6, 2020 •

edited