search

openshift / ocm-container

Containerized environment for accessing OpenShift v4 clusters, packing necessary tools/scripts
Apache License 2.0
10 stars 63 forks source link

Add settings for repo and tag #207

Closed rendhalver closed 5 months ago

rendhalver commented 1 year ago

Adding an way to configure which repo to use and an extra way to set tag using env vars

I have been using direnv as a way to set tag and repo for ocm-container this method will also work with env.source so I thought I would share it. We currently don't have a way to configure which repo to use so this will aslo allow us to use the pre-built images on quay.

clcollins commented 1 year ago

/hold

For discussion in #forum-ocm-container about if this is needed or not

iamkirkbater commented 1 year ago

I like the idea. I'm +1 to this - but also only if we add a -i | --image flag to the CLI so that you can change it on each invocation without using env vars. We already have the similar functionality for -t | --tag that I use heavily when developing and/or A/B testing images locally, so it makes sense to expand this functionality to the image itself.

rendhalver commented 1 year ago

I like the idea. I'm +1 to this - but also only if we add a -i | --image flag to the CLI so that you can change it on each invocation without using env vars. We already have the similar functionality for -t | --tag that I use heavily when developing and/or A/B testing images locally, so it makes sense to expand this functionality to the image itself.

Should we use image or just use -r | --repo to set the container repo and leave the image as ocm-container?

iamkirkbater commented 1 year ago

Should we use image or just use -r | --repo to set the container repo and leave the image as ocm-container?

I could go either way on this - --repo would work and is already implemented, but --image could also be just as flexible, as someone could pass whatever repo/image name they wanted there while it would default to localhost/ocm-container.

I won't block the PR on this point, but I think --image would be more flexible in the future without locking us into a "contract" with the --repo flag in the future if we ever want to change the name of the image itself, as removing that flag would be a breaking change and could cause UX disruption.

I'll let you decide if you want to make the change or leave it as is.

iamkirkbater commented 1 year ago

/approve

clcollins commented 1 year ago

I thought this PR was not needed due to the discussion here: https://redhat-internal.slack.com/archives/C01E0BC9550/p1690501322328629

This is adding extra steps and code we need to maintain when the user can just pull the image and be done with it.

rendhalver commented 1 year ago

I thought this PR was not needed due to the discussion here: https://redhat-internal.slack.com/archives/C01E0BC9550/p1690501322328629

This is adding extra steps and code we need to maintain when the user can just pull the image and be done with it.

I agree that there other ways to achieve this. I have tried a few and this seemed like the best way to do it without impacting how other people use ocm-container.

Maybe some explanation on how I am using this will help. I am a firm believer in speeding up how I do things.

I use direnv a lot to automate a bunch of stuff I do. ocm-container is one of the things I use it for. I am using direnv to set the tag to use based on which directory I am in or globally if I want to run a test version for a while to see how it runs. I tried doing this with aliases but it's clunky and requires reloading my shell or reloading my bash aliases each time I change it, direnv let's me do this without doing reloading.

As for future maintenance, if there is a more efficient way to do this I am open to suggestions. The main bit I require is a way to set tags and repos with env vars. I am not concerned with how that works.

openshift-ci[bot] commented 10 months ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: iamkirkbater, rendhalver

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/ocm-container/blob/master/OWNERS)~~ [iamkirkbater,rendhalver] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
openshift-bot commented 6 months ago

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot commented 5 months ago

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten. Rotten issues close after an additional 30d of inactivity. Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten /remove-lifecycle stale

openshift-merge-robot commented 5 months ago

PR needs rebase.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
rendhalver commented 5 months ago

/close This will not be needed once our golang cli is done.

openshift-ci[bot] commented 5 months ago

@rendhalver: Closed this PR.

In response to [this](https://github.com/openshift/ocm-container/pull/207#issuecomment-2046249702): >/close >This will not be needed once our golang cli is done. Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.