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: add --why flag for zarf dev find-images #2309

Merged
merged 23 commits into from
Mar 6, 2024

Conversation

waveywaves
Copy link
Contributor

@waveywaves waveywaves commented Feb 16, 2024

Description

Add a --why flag for zarf dev find-images

Related Issue

Fixes #2272

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

Copy link

netlify bot commented Feb 16, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 260ed65
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/65e742b3efac590009670691

@waveywaves
Copy link
Contributor Author

Partial (relevant) test output

$ build/zarf-mac-apple dev find-images examples/dos-games --why defenseunicorns/zarf-game:multi-tile-dark --tmpdir /var/folders/v7/yqyq_d6x2996hfnnwvgs5f080000gn/T/zarf-1881051164

 NOTE  Using config file /Users/vibhavbobade/go/src/github.com/defenseunicorns/zarf/zarf-config.toml

 NOTE  Saving log file to
       /var/folders/v7/yqyq_d6x2996hfnnwvgs5f080000gn/T/zarf-2024-02-20-20-44-58-2844959790.log

 NOTE  Using build directory examples/dos-games
  ✔  Looking for images in component "baseline" across 3 resources              
  ✔  Looking up cosign artifacts for discovered images (0/1)                    
component: baseline
manifests: multi-games
resources: 

apiVersion: apps/v1
kind: Deployment
metadata:
  name: game
spec:
  selector:
    matchLabels:
      app: game
  template:
    metadata:
      labels:
        app: game
    spec:
      containers:
      - image: defenseunicorns/zarf-game:multi-tile-dark
        name: multi-game
        ports:
        - containerPort: 8000
          name: http
          protocol: TCP
        resources:
          limits:
            cpu: 250m
            memory: 128Mi
          requests:
            cpu: 50m
            memory: 32Mi

@AustinAbro321
Copy link
Contributor

Currently the code will output images not marked with the --why flag with an empty resources block. I believe the intended behavior is to only show images marked by the --why flag, but @Racer159 can correct me as the issue author.

Output from examples/manifests with ../../build/zarf dev find-images --why=httpd:alpine3.18

component: httpd-local
manifests: simple-httpd-deployment
resources: 

apiVersion: apps/v1
kind: Deployment
metadata:
  name: httpd-deployment
spec:
  replicas: 2
  selector:
    matchLabels:
      app: httpd
  template:
    metadata:
      labels:
        app: httpd
    spec:
      containers:
      - image: httpd:alpine3.18
        name: httpd
        ports:
        - containerPort: 80

component: nginx-remote
manifests: simple-nginx-deployment
resources: 
component: podinfo-kustomize
manifests: simple-podinfo-deployment
resources: 

src/test/e2e/13_find_images_test.go Outdated Show resolved Hide resolved
src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
@waveywaves
Copy link
Contributor Author

I am going to make the following changes

@waveywaves
Copy link
Contributor Author

waveywaves commented Feb 21, 2024

Updated the PR based on the reviews @Racer159 @lucasrod16 @AustinAbro321

src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
Copy link
Contributor

@lucasrod16 lucasrod16 left a comment

Choose a reason for hiding this comment

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

@waveywaves there are some merge conflicts that will need to be resolved

@waveywaves
Copy link
Contributor Author

@waveywaves there are some merge conflicts that will need to be resolved

@lucasrod16 I will do the needful

@waveywaves
Copy link
Contributor Author

waveywaves commented Feb 22, 2024

seems like the k3d test is flaking, can we restart that test ?
@lucasrod16 updated the PR based on the changes you proposed

Add --why flag to `zarf dev find-images` which takes a image tag as an argument.
This command with the `why` flag will output the component, manifest/chart name and the
yaml which matches the given image.

Signed-off-by: Vibhav Bobade <vibhav.bobde@gmail.com>
@waveywaves
Copy link
Contributor Author

waveywaves commented Feb 23, 2024

the e2e-test-with-cluster test failed twice at a single kiwix data injection test against a minikube cluster
first time the test failed because zarf could not connect to the cluster
and right now it has failed because the test could not find a pod which was successfully deployed as seen in the logs below

https://github.com/defenseunicorns/zarf/actions/runs/8013856761/job/21891629494?pr=2309#step:8:5575

running the same possibly flakey TestDataInjection passes locally with minikube

--- PASS: TestDataInjection (47.62s)
PASS
ok      github.com/defenseunicorns/zarf/src/test/e2e    48.707s

src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
Co-authored-by: Lucas Rodriguez <lucas.rodriguez9616@gmail.com>
Copy link
Contributor

@lucasrod16 lucasrod16 left a comment

Choose a reason for hiding this comment

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

Few minor tweaks. Otherwise, this looks good to me!

src/test/e2e/13_find_images_test.go Outdated Show resolved Hide resolved
src/test/e2e/13_find_images_test.go Outdated Show resolved Hide resolved
src/test/e2e/13_find_images_test.go Outdated Show resolved Hide resolved
src/test/e2e/13_find_images_test.go Show resolved Hide resolved
@waveywaves
Copy link
Contributor Author

@lucasrod16 updated the PR

lucasrod16
lucasrod16 previously approved these changes Feb 29, 2024
Copy link
Contributor

@lucasrod16 lucasrod16 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

One small bug, but otherwise super close!

src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
src/pkg/packager/prepare.go Outdated Show resolved Hide resolved
Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
waveywaves and others added 3 commits March 2, 2024 02:58
waveywaves and others added 3 commits March 5, 2024 02:09
…xpected.txt

Co-authored-by: Lucas Rodriguez <lucas.rodriguez9616@gmail.com>
Co-authored-by: Lucas Rodriguez <lucas.rodriguez9616@gmail.com>
src/test/e2e/13_find_images_test.go Outdated Show resolved Hide resolved
waveywaves and others added 2 commits March 5, 2024 16:24
Co-authored-by: Wayne Starr <Racer159@users.noreply.github.com>
Copy link
Contributor

@Racer159 Racer159 left a comment

Choose a reason for hiding this comment

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

lgtm

@Noxsios Noxsios merged commit 5efcb35 into zarf-dev:main Mar 6, 2024
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a --why to zarf dev find-images to show the chart and resource that generated the image reference
5 participants