-
Notifications
You must be signed in to change notification settings - Fork 170
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
Run local development and PR E2E in podman #2817
Conversation
Please rebase pull request. |
Please rebase pull request. |
876bc4c
to
fee0a5c
Compare
e62087b
to
94cd910
Compare
Please rebase pull request. |
94cd910
to
0d146dd
Compare
Please rebase pull request. |
0d146dd
to
0ea812d
Compare
29be917
to
514acd1
Compare
/azp run e2e |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run e2e |
Azure Pipelines successfully started running 1 pipeline(s). |
514acd1
to
088e109
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all making sense to me, can't wait!!
}), | ||
steps.Action(func(context.Context) error { return m.createSecrets(ctx, doc, sub) }), | ||
steps.Action(func(context.Context) error { return m.startContainer(ctx, version) }), | ||
steps.Condition(m.containerFinished, 60*time.Minute, false), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
followup idea: do we need a 60m timeout now? I think that hive install times are typically 15-20 minutes once the container is up, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except in eastus2 :')
Co-authored-by: Jeremy Facchetti <[email protected]>
088e109
to
8661e32
Compare
build-tags: | ||
- "aro" | ||
- "containers_image_openpgp" | ||
- "exclude_graphdriver_devicemapper" | ||
- "exclude_graphdriver_btrfs" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this have a reason to be in this PR? or is it just some extra change bundled in it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Podman's bindings vendors in more of the image lib that causes build dep side effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if these changes belong in this pr too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above, these flags need to be changed. I decided to clean it up a smidge, since it will make the diff when the aro build tag is removed a little bit smaller.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we use ginkgo and gomega here? these are standard unit tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use gomega in some other places, and I've got mildly sick of dealing with stdlib golang testing, so I just wrote it in ginkgo instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about ginkgo? We only use it in e2e, we should have some consistency, we should either use it everywhere in pkg
or not at all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain why ginkgo and gomega are needed here? a comment would be nice in the file, since we are only using those in the e2e and not in pkg
Which issue this PR addresses:
Fixes https://issues.redhat.com/browse/ARO-2532
What this PR does / why we need it:
Runs local development and PR E2E in podman, instead of directly. Removes the remaining Installer dependencies.
Test plan for issue:
Local testing, E2E.
Is there any documentation that needs to be updated for this PR?
Some local dev documentation likely needs revising.