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

feat: warn users of diff architecture image running #1649

Merged
merged 1 commit into from
Oct 31, 2023
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
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
h4ck3rk3y marked this conversation as resolved.
Show resolved Hide resolved
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
}