🌱 Collect kind logs when e2e bootstrap fails #5910
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
| @@ -218,6 +218,7 @@ func setupBootstrapCluster(config *clusterctl.E2EConfig, scheme *runtime.Scheme, | |||
| RequiresDockerSock: config.HasDockerProvider(), | |||
| Images: config.Images, | |||
| IPFamily: config.GetVariable(IPFamily), | |||
| LogFolder: filepath.Join(artifactFolder, "kind"), | |||
There was a problem hiding this comment.
Ideally, this change should be implemented by everyone using the E2E test framework.
An option to give people room to this change overtime is to log only if the log folder is defined.
There was a problem hiding this comment.
Right now the field is not required but if the cluster creation fails it would log into folder "", right?
I would prefer one of the following options:
- enforce that the log folder is set (via an
Expectat the beginning ofCreateKindBootstrapClusterAndLoadImages) - only export logs if the LogFolder is set
Both options are fine for me, I would prefer 1.. Afaik our API compatibility guarantees for framework allow this and as log folder is used everywhere it shouldn't be a problem for users to also set it here. Option 2. would probably mean that apart from us probably almost nobody benefits of this improvement.
(Would be good to mention this in the migration guide if we implement Option 1 though)
There was a problem hiding this comment.
We are now exporting logs only if the LogFolder is set; this is documented in the migration guide (even if it is not mandatory)
| @@ -218,6 +218,7 @@ func setupBootstrapCluster(config *clusterctl.E2EConfig, scheme *runtime.Scheme, | |||
| RequiresDockerSock: config.HasDockerProvider(), | |||
| Images: config.Images, | |||
| IPFamily: config.GetVariable(IPFamily), | |||
| LogFolder: filepath.Join(artifactFolder, "kind"), | |||
There was a problem hiding this comment.
Right now the field is not required but if the cluster creation fails it would log into folder "", right?
I would prefer one of the following options:
- enforce that the log folder is set (via an
Expectat the beginning ofCreateKindBootstrapClusterAndLoadImages) - only export logs if the LogFolder is set
Both options are fine for me, I would prefer 1.. Afaik our API compatibility guarantees for framework allow this and as log folder is used everywhere it shouldn't be a problem for users to also set it here. Option 2. would probably mean that apart from us probably almost nobody benefits of this improvement.
(Would be good to mention this in the migration guide if we implement Option 1 though)
fabriziopandini
force-pushed
the
e2e-collect-kind-logs
branch
from
bb702e3 to
b5305aa
Compare
fabriziopandini
force-pushed
the
e2e-collect-kind-logs
branch
from
b5305aa to
2fe632f
Compare
|
Thx! /lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vincepri 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 |
k8s-ci-robot
added
the
approved
This PR adds LogFolder Parameter in e2e for collecting kind logs. Please see kubernetes-sigs/cluster-api#5910 for reference.
What this PR does / why we need it:
This PR improves the E2E test framework by collecting kind logs in case creating the bootstrap cluster fails for any reason