Skip to content

Update logrus to v1.0.1#2334

Closed
dmcgowan wants to merge 2 commits intomoby:masterfrom
dmcgowan:update-logrus
Closed

Update logrus to v1.0.1#2334
dmcgowan wants to merge 2 commits intomoby:masterfrom
dmcgowan:update-logrus

Conversation

@dmcgowan
Copy link
Member

@dmcgowan dmcgowan commented Jul 26, 2017 •
edited
Loading

Fixes case sensitivity issue.

See https://github.com/sirupsen/logrus#logrus-

@dmcgowan
Update logrus to v1.0.1
c12059f 
Fixes case sensitivity issue

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@dmcgowan
Copy link
Member Author

dmcgowan commented Jul 26, 2017

Will re-open once some of the dependencies have been fixed

@dmcgowan dmcgowan closed this Jul 26, 2017
@aaronlehmann
Copy link
Collaborator

aaronlehmann commented Jul 26, 2017

I am seeing the same vendor errors on master. It seems like our use of some vendored packages has changed, so we need to revendor those to delete unused subpackages and make vndr happy. Could you please do that in this PR?

I assume this change won't cause problems vendoring swarmkit into projects that still use big-S Sirupsen?

@aaronlehmann
Copy link
Collaborator

aaronlehmann commented Jul 26, 2017

(Sorry, it's actually files that vndr wants to add. I can open a separate PR to do this if you prefer.)

@dmcgowan dmcgowan mentioned this pull request Jul 26, 2017
Merged
5 tasks
@dmcgowan dmcgowan reopened this Aug 3, 2017
@dmcgowan
Update vendors
bfed21f 
Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@liron-l liron-l mentioned this pull request Aug 3, 2017
Merged
@aaronlehmann
Copy link
Collaborator

aaronlehmann commented Aug 3, 2017

@dmcgowan: Are you using the latest version of vndr? I remember having a similar problem when my copy was out-of-date.

@ijc ijc mentioned this pull request Aug 3, 2017
Closed
@ijc
Copy link
Contributor

ijc commented Aug 3, 2017

@dmcgowan wrt bumping containerd to pickup the new case, with this PR merged onto #2299 the bump to alpha2 then alpha3 was pretty trivial (i.e. trivial enough you could just fold it in rather than create your own branch of containerd):

For the alpha2 update it is just s/WithNewRootFS/WithNewSnapshot/ in one place (diff here extracted from git show of the merge commit, so a bit more complex than the actual change):

diff --cc agent/exec/containerd/adapter.go
index b44f4f14,fd9ee819..08405381
--- a/agent/exec/containerd/adapter.go
+++ b/agent/exec/containerd/adapter.go
@@@ -283,3 -239,3 +283,3 @@@ func (c *containerAdapter) prepare(ctx 
                containerd.WithSpec(spec),
--              containerd.WithNewRootFS(c.name, c.image))
++              containerd.WithNewSnapshot(c.name, c.image))
        if err != nil {
diff --cc vendor.conf
index 45c0cb12,8bd0a799..36cf7cd3
--- a/vendor.conf
+++ b/vendor.conf
@@@ -33,5 -33,5 +33,5 @@@ github.com/opencontainers/image-spec 87
  # containerd executor
- github.com/containerd/containerd df14b2c75b7eb7bbda9c9520faba6602a5060cf5 # v1.0.0-alpha1
 -github.com/containerd/containerd 70d8429b4096a8cd7843014e795207cdbc73d249 https://github.com/dmcgowan/containerd.git
 -github.com/containerd/fifo 69b99525e472735860a5269b75af1970142b3062
 -github.com/opencontainers/runtime-spec v1.0.0-rc5
++github.com/containerd/containerd 856b03843737a45ba1b8c1bf9558e44cabedb2e7# v1.0.0-alpha2
 +github.com/containerd/fifo fbfb6a11ec671efbe94ad1c12c2e98773f19e1e6
 +github.com/opencontainers/runtime-spec 4ffc6820f422b93b16e68681104c225c61b066c7 # v1.0.0
  golang.org/x/sync 450f422ab23cf9881c94e2db30cac0eb1b7cf80c

and for the alpha3 it is just s/TaskStatus/Status/ in one place (a normal diff this time):

diff --git a/agent/exec/containerd/adapter.go b/agent/exec/containerd/adapter.go
index 08405381..cacf2589 100644
--- a/agent/exec/containerd/adapter.go
+++ b/agent/exec/containerd/adapter.go
@@ -360,7 +360,7 @@ func (c *containerAdapter) wait(ctx context.Context) error {
 type status struct {
        ID         string
        Pid        uint32
-       Status     containerd.TaskStatus
+       Status     containerd.Status
        ExitStatus error
 }
 
diff --git a/vendor.conf b/vendor.conf
index 36cf7cd3..911b04dd 100644
--- a/vendor.conf
+++ b/vendor.conf
@@ -31,7 +31,8 @@ github.com/opencontainers/go-digest a6d0ee40d4207ea02364bd3b9e8e77b9159ba1eb
 github.com/opencontainers/image-spec 87998cd070d9e7a2c79f8b153a26bea0425582e5 # v1.0.0
 
 # containerd executor
-github.com/containerd/containerd 856b03843737a45ba1b8c1bf9558e44cabedb2e7# v1.0.0-alpha2
+github.com/containerd/containerd 0fa76584f8360345fdedccf4689ea1c8908a1f9d# v1.0.0-alpha3
+github.com/containerd/continuity 86cec1535a968310e7532819f699ff2830ed7463
 github.com/containerd/fifo fbfb6a11ec671efbe94ad1c12c2e98773f19e1e6
 github.com/opencontainers/runtime-spec 4ffc6820f422b93b16e68681104c225c61b066c7 # v1.0.0
 golang.org/x/sync 450f422ab23cf9881c94e2db30cac0eb1b7cf80c

Scratch branch at https://github.com/ijc/swarmkit/tree/containerd-logrus. The bump to alpha2 is actually in the merge. I tested the HEAD there briefly and it seemed to work ok.

@simonferquel simonferquel mentioned this pull request Aug 9, 2017
Merged
@thaJeztah
Copy link
Member

thaJeztah commented Aug 12, 2017

what's the status on this one?

@aaronlehmann
Copy link
Collaborator

aaronlehmann commented Aug 12, 2017

I believe it's superseded by #2345

@thaJeztah
Copy link
Member

thaJeztah commented Aug 12, 2017

Ah, thanks! Just hoping all of these PR's are merged before the next release is cut off, otherwise it may be messy 😅

@aaronlehmann
Copy link
Collaborator

aaronlehmann commented Aug 12, 2017

Interesting that I can close pull requests but not merge.

@thaJeztah
Copy link
Member

thaJeztah commented Aug 12, 2017

I have the same; it's because of the way the "protected branches" are configured; IIRC, only "owners" are allowed to merge changes to master (I can edit comments, review PR's, close/open PR's and issues, but am not allowed to merge)

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.

4 participants

@dmcgowan @aaronlehmann @ijc @thaJeztah