-
-
Notifications
You must be signed in to change notification settings - Fork 504
Description
@iwilltry42 I'm seeing this issue on ubuntu when upgrading from k3d 5.3.0 to 5.4.1.
I'm seeing output like:
INFO[0000] Importing image(s) into cluster '$CLUSTER_NAME'
INFO[0000] Starting new tools node...
INFO[0000] Starting Node 'k3d-$CLUSTER_NAME-tools'
INFO[0000] Saving 3 tarball(s) to shared image volume...
INFO[0000] Importing images into nodes...
INFO[0000] Importing images from tarball '/k3d/images/k3d-$CLUSTER_NAME-images-$IMAGE1.tar' into node 'k3d-$CLUSTER_NAME-server-0'...
INFO[0000] Importing images from tarball '/k3d/images/k3d-$CLUSTER_NAME-images-$IMAGE2.tar' into node 'k3d-$CLUSTER_NAME-server-0'...
INFO[0000] Importing images from tarball '/k3d/images/k3d-$CLUSTER_NAME-images-$IMAGE3.tar' into node 'k3d-$CLUSTER_NAME-server-0'...
ERRO[0001] failed to import images in node 'k3d-$CLUSTER_NAME-server-0': Exec process in node 'k3d-$CLUSTER_NAME-server-0' failed with exit code '1'
ERRO[0001] failed to import images in node 'k3d-$CLUSTER_NAME-server-0': Exec process in node 'k3d-$CLUSTER_NAME-server-0' failed with exit code '1'
ERRO[0001] failed to import images in node 'k3d-$CLUSTER_NAME-server-0': Exec process in node 'k3d-$CLUSTER_NAME-server-0' failed with exit code '1'
INFO[0001] Removing the tarball(s) from image volume...
INFO[0002] Removing k3d-tools node...
INFO[0003] Successfully imported image(s)
INFO[0003] Successfully imported 3 image(s) into 1 cluster(s)
One thing that jumps out: the code in importWithToolsNode() that logs these failures when copying images swallows errors without returning them to the caller:
Lines 125 to 127 in 852df77
| if err := runtime.CopyToNode(ctx, file, tarName, toolsNode); err != nil { | |
| l.Log().Errorf("failed to copy image tar '%s' to tools node! Error below:\n%+v", file, err) | |
| continue |
Compare that to this other k3d code to import images into clusters (rather than nodes) which fails if any individual image fails to import:
Lines 75 to 86 in 7b1b416
| errOccurred := false | |
| for _, cluster := range clusters { | |
| l.Log().Infof("Importing image(s) into cluster '%s'", cluster.Name) | |
| if err := client.ImageImportIntoClusterMulti(cmd.Context(), runtimes.SelectedRuntime, images, cluster, loadImageOpts); err != nil { | |
| l.Log().Errorf("Failed to import image(s) into cluster '%s': %+v", cluster.Name, err) | |
| errOccurred = true | |
| } | |
| } | |
| if errOccurred { | |
| l.Log().Warnln("At least one error occured while trying to import the image(s) into the selected cluster(s)") | |
| os.Exit(1) | |
| } |
Or even this code in the same importWithToolsNode() method that returns errors if any image fails to save:
Lines 114 to 116 in 852df77
| if err := runtime.ExecInNode(ctx, toolsNode, append([]string{"./k3d-tools", "save-image", "-d", tarName}, imagesFromRuntime...)); err != nil { | |
| return fmt.Errorf("failed to save image(s) in tools container for cluster '%s': %w", cluster.Name, err) | |
| } |
Should importWithToolsNode() return an error whenever any image installations encounter an error? That would at least mean that image installation errors are reported as overall install errors: that doesn't fix the install errors themselves, but it seems more appropriate than treating them as successes.
Originally posted by @CodingCanuck in #954 (comment)