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

Don't verify redhat-appstudio/dance-bootstrap-app #1271

Closed

Conversation

simonbaird
Copy link
Contributor

@simonbaird simonbaird commented Aug 8, 2024

This is the upstream change related to the following downstream changes to make sure everything stays in sync.

Explanation: For various reasons quay.io/redhat-appstudio/dance-bootstrap-app will never pass an EC check, so try to validate it.

This was causing problems during QE testing of the Jenkins pipeline in RHTAP 1.2.

Ref: https://issues.redhat.com/browse/RHTAPBUGS-1284

@simonbaird
Copy link
Contributor Author

Hmm, it says "Remove quotes from right-hand side of =~ to match as a regex rather than literally" but I kinda want it to match literally.

I'll see if I can convert it to regex..

This is the upstream change related to the following downstream changes to
make sure everything stays in sync.

- redhat-appstudio/tssc-sample-jenkins#5
- redhat-appstudio/tssc-sample-pipelines#54

Explanation: For various reasons quay.io/redhat-appstudio/dance-bootstrap-app
will never pass an EC check, so try to validate it.

This was causing problems during QE testing of the Jenkins pipeline in RHTAP
1.2.

Ref: https://issues.redhat.com/browse/RHTAPBUGS-1284
@simonbaird simonbaird force-pushed the rhtap-gather-images-tweak branch from e580ce6 to 0609064 Compare August 8, 2024 22:38
@simonbaird
Copy link
Contributor Author

Seems to work fine without the quotes 🤷 .

jduimovich

This comment was marked as duplicate.

Copy link
Contributor

@jduimovich jduimovich 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

@chmeliik chmeliik left a comment

Choose a reason for hiding this comment

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

Unless the user somehow "promotes" the bootstrap image to stage or prod, the promotion pipeline will not try to validate the bootstrap image. Why is this change needed?

@chmeliik
Copy link
Contributor

Hmm, it says "Remove quotes from right-hand side of =~ to match as a regex rather than literally" but I kinda want it to match literally.

I'll see if I can convert it to regex..

Just FYI: to match literally, you want = rather than =~

@simonbaird
Copy link
Contributor Author

Hmm, it says "Remove quotes from right-hand side of =~ to match as a regex rather than literally" but I kinda want it to match literally.
I'll see if I can convert it to regex..

Just FYI: to match literally, you want = rather than =~

I want it to match with :latest or @sha256:abcdef123 at the end, so that's why I'm chosing a regex. (Could maybe use == "quay.io/redhat-appstudio/dance-bootstrap-app"* ]] to achieve that, but I think =~ is fine.)

@chmeliik
Copy link
Contributor

Unless the user somehow "promotes" the bootstrap image to stage or prod, the promotion pipeline will not try to validate the bootstrap image. Why is this change needed?

After looking at the Jenkins pipeline briefly, I think the root issue is that it doesn't set the TARGET_BRANCH variable so the task doesn't look for changed images, it always returns all the images. https://github.com/redhat-appstudio/tssc-sample-templates/blob/bed5fcaf8eb225dcf5ab17b09a4fb3be8dad4309/skeleton/ci/gitops-template/jenkins/rhtap/env.sh#L32

- name: TARGET_BRANCH
description: >
If specified, will gather only the images that changed between
the current revision and the target branch. Useful for pull requests.
Note that the repository cloned on the source workspace must already
contain the origin/$TARGET_BRANCH reference.

So I don't think we need the "ignore dance-boostrap-app" workaround upstream

@simonbaird
Copy link
Contributor Author

Thanks @chmeliik , I think you're probably right. I'm happy to abandon this and also redhat-appstudio/tssc-sample-pipelines#54 .

@simonbaird simonbaird marked this pull request as draft August 12, 2024 16:32
@simonbaird simonbaird closed this Aug 26, 2024
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.

3 participants