🌱 make: add non-kind shared and sharded e2e #2265
Conversation
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
You can't prove a negative, and eventually consistent systems don't like to conform to your idea of "long enough" before synchronizing. We can't assume that "nothing will happen" within 5s here. If we wanted to make sure that the controller had seen & acknowledged the object, we should have it post some observed generation. Since these polls were not effective at best, and flaky at worst, we can save 10s by just making one call. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
stevekuznetsov
force-pushed
the
skuznets/non-kind-e2e
branch
from
437b38d
to
d512fe7
Compare
|
/lgtm |
|
Running KCP Error: cannot create the 'admin.kubeconfig` file with an empty token for the shard-admin user
error: kcp shard kcp terminated with exit code 1 |
|
@nrb that's a pre-existing boo-boo for these targets |
First, we don't want to pass `-count 1` to `go test` unless someone explicitly opts into this behavior, as this explicitly invalidates the test cache. When re-running tests locally, being able to run the full suite every time and use the cache is incredibly useful. Second, we should only bother with limiting parallelism when we're starting up a full `kcp` and `etcd` server per test case. Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
stevekuznetsov
force-pushed
the
skuznets/non-kind-e2e
branch
from
d512fe7
to
8e53e07
Compare
| } | ||
| require.NoError(t, err) | ||
| return false | ||
| }, 5*time.Second, time.Second, "upstream Deployment %s/%s got deleted or there was an error", upstreamNamespace.Name, upstreamDeployment.Name) |
There was a problem hiding this comment.
I think the intent here was to wait a ~short amount of time to make sure there weren't any unintentional/unexpected deletions that trickled in after a period of time.
There was a problem hiding this comment.
I wrote in the commit why that is not a valid way to test this behavior.
| @@ -287,10 +288,16 @@ func TestClusterWorkspaceTypes(t *testing.T) { | |||
| }) | |||
|
|
|||
| t.Logf("Expect workspace to be stuck in initializing phase") | |||
| time.Sleep(5 * time.Second) | |||
There was a problem hiding this comment.
This is another case where there's a desire to wait a ~short amount of time and make sure an event does not happen. Not sure how we want to approach those.
There was a problem hiding this comment.
It is not - we are waiting to see it be in initializing, no?
There was a problem hiding this comment.
My comment about proving negatives and assuming time-scales for eventually consistent systems applies to this case as well.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ncdc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
test/e2e: make sure tests actually run in parallel
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
test/e2e: be consistent about timeouts and poll intervals
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
test/e2e/workspacetype: poll, don't blindly wait for 5s
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
test/e2e/syncer: send one request, not many
You can't prove a negative, and eventually consistent systems don't like
to conform to your idea of "long enough" before synchronizing. We can't
assume that "nothing will happen" within 5s here. If we wanted to make
sure that the controller had seen & acknowledged the object, we should
have it post some observed generation.
Since these polls were not effective at best, and flaky at worst, we can
save 10s by just making one call.
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
make: add non-kind variants of shared & sharded e2e
Signed-off-by: Steve Kuznetsov skuznets@redhat.com
make: be more intelligent with parallelism, count
First, we don't want to pass
-count 1togo testunless someoneexplicitly opts into this behavior, as this explicitly invalidates the
test cache. When re-running tests locally, being able to run the full
suite every time and use the cache is incredibly useful.
Second, we should only bother with limiting parallelism when we're
starting up a full
kcpandetcdserver per test case.Signed-off-by: Steve Kuznetsov skuznets@redhat.com
/cc @jmprusi @davidfestal
/assign @ncdc @sttts