Bump SwarmKit to f74983e7c015a38a81c8642803a78b8322cf7eac #36274
Conversation
- Replace EC Private Key with PKCS#8 PEMs - Fix IP overlap with empty EndpointSpec - Add support for Support SCTP port mapping (depends on changes in libnetwork) - [orchestrator/updater] Do not reschedule tasks if only placement constraints change and are satisfied by the assigned node - Ensure task reaper stopChan is closed no more than once - [manager/dispatcher] Synchronization fixes Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
looks like several tests are failing |
|
Failures are due to the changes in moby/swarmkit#2246, so the tests may need some updating. Talked with @cyli on Slack, and she'll assist with updating the tests 👍 |
|
@thaJeztah Tested this locally, and the swarm lock/unlock tests that failed all pass with this change: diff --git a/integration-cli/docker_cli_swarm_test.go b/integration-cli/docker_cli_swarm_test.go
index 283f576f5..4a5ec9a56 100644
--- a/integration-cli/docker_cli_swarm_test.go
+++ b/integration-cli/docker_cli_swarm_test.go
@@ -4,7 +4,6 @@ package main
import (
"bytes"
- "crypto/x509"
"encoding/json"
"encoding/pem"
"fmt"
@@ -25,6 +24,7 @@ import (
"github.com/docker/libnetwork/driverapi"
"github.com/docker/libnetwork/ipamapi"
remoteipam "github.com/docker/libnetwork/ipams/remote/api"
+ "github.com/docker/swarmkit/ca/keyutils"
"github.com/go-check/check"
"github.com/gotestyourself/gotestyourself/fs"
"github.com/gotestyourself/gotestyourself/icmd"
@@ -1007,7 +1007,7 @@ func checkKeyIsEncrypted(d *daemon.Swarm) func(*check.C) (interface{}, check.Com
return fmt.Errorf("invalid PEM-encoded private key"), nil
}
- return x509.IsEncryptedPEMBlock(keyBlock), nil
+ return keyutils.IsEncryptedPEMBlock(keyBlock), nil
}
}Basically the PKCS8 changes updated the encryption on the keys so that the |
The PKCS8 changes updated the encryption on the keys so that the `x509.IsEncryptedPEMBlock` may no longer return true because it cannot parse the PEM block. The `keyutils` module in SwarmKit can tell whether it is encrypted either way. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
|
Applied the patch; let's see if CI goes green now 👍 |
There was a problem hiding this comment.
Looks like this patch is pulling in a change from moby/swarmkit#2495 that I'm a bit suspect of. The usage of the waitgroup looks racey on first glance.
| // Note that Stop() first does Dispatcher.ctx.cancel() followed by | ||
| // shutdownWait.Wait() to make sure new rpc's don't start before waiting | ||
| // for existing ones to finish. | ||
| d.shutdownWait.Add(1) |
There was a problem hiding this comment.
This looks suspect to me.
A waitgroup Add() is not safe to do async.
Ping @anshulpundir
There was a problem hiding this comment.
sync.WaitGroup operations should be atomic. I think this is a false alarm. I will try to follow up with the community.
FYI: https://groups.google.com/forum/#!topic/golang-nuts/Qq_h0_M51YM
There was a problem hiding this comment.
Actually it seems wg.Wait() must be synchronized with the first wg.Add(). While this should be OK from the code, I'll look at how to address this for the CI failures.
There was a problem hiding this comment.
@nishanttotla and I discussed the data race failure in unit-tests and it should be OK to move forward. Because of how the waitgroup is used, the race should not have any undesired side-effects.
|
Looks like moby/swarmkit#2495 is being reverted in moby/swarmkit#2518 |
Full diff:
moby/swarmkit@68a376d...f74983e
ping @anshulpundir @nishanttotla PTAL