Skip to content

Commit

Permalink
feat: warn users of diff architecture image running (#1649)
Browse files Browse the repository at this point in the history
## Description:
We now warn users in the beginning if the image is different from whats
expected

## Is this change user facing?
YES/NO
<!-- If yes, please add the "user facing" label to the PR -->
<!-- If yes, don't forget to include docs changes where relevant -->

## References (if applicable):
<!-- Add relevant Github Issues, Discord threads, or other helpful
information. -->
  • Loading branch information
h4ck3rk3y authored Oct 31, 2023
1 parent 86ceeaa commit 77f2f69
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func NewDockerKurtosisBackend(
}
}

func (backend *DockerKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, error) {
func (backend *DockerKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
return backend.dockerManager.FetchImage(ctx, image, downloadMode)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ func (manager *DockerManager) CreateAndStartContainer(
dockerImage = dockerImage + dockerTagSeparatorChar + dockerDefaultTag
}

_, err := manager.FetchImage(ctx, dockerImage, args.imageDownloadMode)
_, _, err := manager.FetchImage(ctx, dockerImage, args.imageDownloadMode)
if err != nil {
return "", nil, stacktrace.Propagate(err, "An error occurred fetching image '%v'", dockerImage)
}
Expand Down Expand Up @@ -1247,7 +1247,7 @@ func (manager *DockerManager) FetchLatestImage(ctx context.Context, dockerImage
return nil
}

func (manager *DockerManager) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, error) {
func (manager *DockerManager) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
var err error
var pulledFromRemote bool = true
logrus.Infof("Fetching image '%s' is running in '%s' mode", image, downloadMode)
Expand All @@ -1258,14 +1258,19 @@ func (manager *DockerManager) FetchImage(ctx context.Context, image string, down
case image_download_mode.ImageDownloadMode_Missing:
pulledFromRemote, err = manager.FetchImageIfMissing(ctx, image)
default:
return false, stacktrace.NewError("Undefined image pulling mode: '%v'", image_fetching)
return false, "", stacktrace.NewError("Undefined image pulling mode: '%v'", image_fetching)
}

if err != nil {
return false, stacktrace.Propagate(err, "An error occurred fetching image '%v'", image)
return false, "", stacktrace.Propagate(err, "An error occurred fetching image '%v'", image)
}

return pulledFromRemote, nil
imageArchitecture, err := manager.getImagePlatform(ctx, image)
if err != nil {
return false, "", stacktrace.Propagate(err, "An error occurred while fetching the architecture of the image")
}

return pulledFromRemote, imageArchitecture, nil
}

func (manager *DockerManager) CreateContainerExec(context context.Context, containerId string, cmd []string) (*types.HijackedResponse, error) {
Expand Down Expand Up @@ -1404,6 +1409,15 @@ func (manager *DockerManager) getNetworksByFilterArgs(ctx context.Context, args
return networks, nil
}

func (manager *DockerManager) getImagePlatform(ctx context.Context, imageName string) (string, error) {
imageInspect, _, err := manager.dockerClient.ImageInspectWithRaw(ctx, imageName)
if err != nil {
return "", stacktrace.Propagate(err, "an error occurred while running image inspect on image '%v'", imageName)
}

return imageInspect.Architecture, nil
}

/*
Creates a Docker-Container-To-Host Port mapping, defining how a Container's JSON RPC and service-specific ports are
mapped to the host ports.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ func NewKubernetesKurtosisBackend(
}
}

func (backend *KubernetesKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, error) {
func (backend *KubernetesKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
logrus.Warnf("FetchImage isn't implemented for Kubernetes yet")
return false, nil
return false, "", nil
}

func (backend *KubernetesKurtosisBackend) PruneUnusedImages(ctx context.Context) ([]string, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ func NewMetricsReportingKurtosisBackend(underlying backend_interface.KurtosisBac
return &MetricsReportingKurtosisBackend{underlying: underlying}
}

func (backend *MetricsReportingKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, error) {
pulledFromRemote, err := backend.underlying.FetchImage(ctx, image, downloadMode)
func (backend *MetricsReportingKurtosisBackend) FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error) {
pulledFromRemote, architecture, err := backend.underlying.FetchImage(ctx, image, downloadMode)
if err != nil {
return false, stacktrace.Propagate(err, "An error occurred pulling image '%v'", image)
return false, "", stacktrace.Propagate(err, "An error occurred pulling image '%v'", image)
}
return pulledFromRemote, nil
return pulledFromRemote, architecture, nil
}

func (backend *MetricsReportingKurtosisBackend) PruneUnusedImages(ctx context.Context) ([]string, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ type KurtosisBackend interface {
// FetchImage always attempts to retrieve the latest image.
// If retrieving the latest [dockerImage] fails, the local image will be used.
// Returns True is it was retrieved from cloud or False if it's a local image
FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, error)
// Returns a string that represents the architecture of the image
FetchImage(ctx context.Context, image string, downloadMode image_download_mode.ImageDownloadMode) (bool, string, error)

PruneUnusedImages(ctx context.Context) ([]string, error)

Expand Down
36 changes: 22 additions & 14 deletions container-engine-lib/lib/backend_interface/mock_kurtosis_backend.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package startosis_engine
import (
"context"
"fmt"
"runtime"
"strings"

"github.com/kurtosis-tech/kurtosis/api/golang/core/kurtosis_core_rpc_api_bindings"
Expand All @@ -26,7 +27,11 @@ const (
containerDownloadedImagesMsgFromLocal = "locally cached"
containerDownloadedImagesMsgFromRemote = "remotely downloaded"
containerDownloadedImagesMsgLineFormat = "> %s - %s"
linebreak = "\n"

containerImageArchWarningHeaderFormat = "Container images with different architecture than expected(%s):"
containerImageArchitectureMsgLineFormat = "> %s - %s"

linebreak = "\n"
)

type StartosisValidator struct {
Expand Down Expand Up @@ -208,13 +213,21 @@ func sendContainerImageSummaryInfoMsg(
}

imageLines := []string{}
imagesWithIncorrectArchLines := []string{}

for image, validatedImage := range imageSuccessfullyValidated {
pulledFromStr := containerDownloadedImagesMsgFromLocal
if validatedImage.GetPulledFromRemote() {
pulledFromStr = containerDownloadedImagesMsgFromRemote
}

architecture := validatedImage.GetArchitecture()

if architecture != runtime.GOARCH {
imageWithIncorrectArchLine := fmt.Sprintf(containerImageArchitectureMsgLineFormat, image, architecture)
imagesWithIncorrectArchLines = append(imagesWithIncorrectArchLines, imageWithIncorrectArchLine)
}

imageLine := fmt.Sprintf(containerDownloadedImagesMsgLineFormat, image, pulledFromStr)
imageLines = append(imageLines, imageLine)
}
Expand All @@ -225,6 +238,14 @@ func sendContainerImageSummaryInfoMsg(
msg := strings.Join(msgLines, linebreak)

starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromInfoMsg(msg)

if len(imagesWithIncorrectArchLines) > 0 {
imageWarningHeader := fmt.Sprintf(containerImageArchWarningHeaderFormat, runtime.GOARCH)
imagesWithArchMsgLines := []string{imageWarningHeader}
imagesWithArchMsgLines = append(imagesWithArchMsgLines, imagesWithIncorrectArchLines...)
imagesWithDiffArchWarningMessage := strings.Join(imagesWithArchMsgLines, linebreak)
starlarkRunResponseLineStream <- binding_constructors.NewStarlarkRunResponseLineFromWarning(imagesWithDiffArchWarningMessage)
}
}

func updateProgressWithDownloadInfo(starlarkRunResponseLineStream chan<- *kurtosis_core_rpc_api_bindings.StarlarkRunResponseLine, imageCurrentlyInProgress []string, numberOfImageValidated uint32, totalNumberOfImagesToValidate uint32) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,17 @@ func (validator *DockerImagesValidator) Validate(ctx context.Context, environmen
func fetchImageFromBackend(ctx context.Context, wg *sync.WaitGroup, imageCurrentlyDownloading chan bool, backend *backend_interface.KurtosisBackend, imageName string, imageDownloadMode image_download_mode.ImageDownloadMode, pullErrors chan<- error, imageDownloadStarted chan<- string, imageDownloadFinished chan<- *ValidatedImage) {
logrus.Debugf("Requesting the download of image: '%s'", imageName)
var imagePulledFromRemote bool
var imageArch string
defer wg.Done()
imageCurrentlyDownloading <- true
imageDownloadStarted <- imageName
defer func() {
<-imageCurrentlyDownloading
imageDownloadFinished <- NewValidatedImage(imageName, imagePulledFromRemote)
imageDownloadFinished <- NewValidatedImage(imageName, imagePulledFromRemote, imageArch)
}()

logrus.Debugf("Starting the download of image: '%s'", imageName)
imagePulledFromRemote, err := (*backend).FetchImage(ctx, imageName, imageDownloadMode)
imagePulledFromRemote, imageArch, err := (*backend).FetchImage(ctx, imageName, imageDownloadMode)
if err != nil {
logrus.Warnf("Container image '%s' download failed. Error was: '%s'", imageName, err.Error())
pullErrors <- startosis_errors.WrapWithValidationError(err, "Failed fetching the required image '%v'.", imageName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package startosis_validator
type ValidatedImage struct {
name string
pulledFromRemote bool
architecture string
}

func NewValidatedImage(name string, pulledFromRemote bool) *ValidatedImage {
return &ValidatedImage{name: name, pulledFromRemote: pulledFromRemote}
func NewValidatedImage(name string, pulledFromRemote bool, architecture string) *ValidatedImage {
return &ValidatedImage{name: name, pulledFromRemote: pulledFromRemote, architecture: architecture}
}

func (v *ValidatedImage) GetName() string {
Expand All @@ -16,3 +17,7 @@ func (v *ValidatedImage) GetName() string {
func (v *ValidatedImage) GetPulledFromRemote() bool {
return v.pulledFromRemote
}

func (v *ValidatedImage) GetArchitecture() string {
return v.architecture
}

0 comments on commit 77f2f69

Please sign in to comment.