Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🌱 Collect kind logs when e2e bootstrap fails #5910

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/book/src/developer/providers/v1.0-to-v1.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ are kept in sync with the versions used by `sigs.k8s.io/controller-runtime`.
* e2e tests:
* `QuickStartSpec` is now able to test clusters using ClusterClass. Please see this [PR](https://github.com/kubernetes-sigs/cluster-api/pull/5423) for an example on how to use it.
* `SelfHostedSpec` is now able to test clusters using ClusterClass. Please see this [PR](https://github.com/kubernetes-sigs/cluster-api/pull/5600) for an example on how to use it.
* Test framework provides better logging in case of failures when creating the bootstrap kind cluster; in order to
fully exploit this feature, it is required to pass the `LogFolder` parameter when calling `CreateKindBootstrapClusterAndLoadImages`. Please see this [PR](https://github.com/kubernetes-sigs/cluster-api/pull/5910) for an example on how to use it.
* The `gci` linter has been enabled to enforce consistent imports. As usual, feel free to take a look at our linter config, but of course it's not mandatory to adopt it.
* The Tilt dev setup has been extended with:
* [an option to deploy Grafana, Loki and promtail](https://github.com/kubernetes-sigs/cluster-api/pull/5336)
Expand Down
1 change: 1 addition & 0 deletions test/e2e/e2e_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. enforce that the log folder is set (via an Expect at the beginning of CreateKindBootstrapClusterAndLoadImages)
  2. 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)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are now exporting logs only if the LogFolder is set; this is documented in the migration guide (even if it is not mandatory)

})
Expect(clusterProvider).ToNot(BeNil(), "Failed to create a bootstrap cluster")

Expand Down
19 changes: 18 additions & 1 deletion test/framework/bootstrap/kind_provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,13 @@ func WithIPv6Family() KindClusterOption {
})
}

// LogFolder implements a New Option that instruct the kindClusterProvider to dump bootstrap logs in a folder in case of errors.
func LogFolder(path string) KindClusterOption {
return kindClusterOptionAdapter(func(k *KindClusterProvider) {
k.logFolder = path
})
}

// NewKindClusterProvider returns a ClusterProvider that can create a kind cluster.
func NewKindClusterProvider(name string, options ...KindClusterOption) *KindClusterProvider {
Expect(name).ToNot(BeEmpty(), "name is required for NewKindClusterProvider")
Expand All @@ -94,6 +101,7 @@ type KindClusterProvider struct {
kubeconfigPath string
nodeImage string
ipFamily clusterv1.ClusterIPFamily
logFolder string
}

// Create a Kubernetes cluster using kind.
Expand Down Expand Up @@ -141,9 +149,18 @@ func (k *KindClusterProvider) createKindCluster() {
nodeImage = k.nodeImage
}
kindCreateOptions = append(kindCreateOptions, kind.CreateWithNodeImage(nodeImage))
kindCreateOptions = append(kindCreateOptions, kind.CreateWithRetain(true))

err := kind.NewProvider(kind.ProviderWithLogger(cmd.NewLogger())).Create(k.name, kindCreateOptions...)
provider := kind.NewProvider(kind.ProviderWithLogger(cmd.NewLogger()))
err := provider.Create(k.name, kindCreateOptions...)
if err != nil {
// if requested, dump kind logs
if k.logFolder != "" {
if err := provider.CollectLogs(k.name, k.logFolder); err != nil {
log.Logf("Failed to collect logs from kind: %v", err)
}
}

errStr := fmt.Sprintf("Failed to create kind cluster %q: %v", k.name, err)
// Extract the details of the RunError, if the cluster creation was triggered by a RunError.
var runErr *exec.RunError
Expand Down
6 changes: 6 additions & 0 deletions test/framework/bootstrap/kind_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ type CreateKindBootstrapClusterAndLoadImagesInput struct {

// IPFamily is either ipv4 or ipv6. Default is ipv4.
IPFamily string

// LogFolder where to dump logs in case of errors
LogFolder string
}

// CreateKindBootstrapClusterAndLoadImages returns a new Kubernetes cluster with pre-loaded images.
Expand All @@ -68,6 +71,9 @@ func CreateKindBootstrapClusterAndLoadImages(ctx context.Context, input CreateKi
if input.IPFamily == "IPv6" {
options = append(options, WithIPv6Family())
}
if input.LogFolder != "" {
options = append(options, LogFolder(input.LogFolder))
}

clusterProvider := NewKindClusterProvider(input.Name, options...)
Expect(clusterProvider).ToNot(BeNil(), "Failed to create a kind cluster")
Expand Down