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

Add default transport to push if not provided #260

Conversation

TomSweeneyRedHat
Copy link
Member

Signed-off-by: TomSweeneyRedHat tsweeney@redhat.com

Currently with 'buildah push' if the transport is not defined, it will fail. This change will retry if a transport was not found by adding "docker://" to the front of the destination image and retrying.

I tweaked a test in the test script to test for this condition and also converted the creation of htpasswd from Docker to Buildah. I tried running the registry with Buildah too, but 'docker login' wasn't happy. More investigation there in a follow up.

Copy link
Member Author

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

Looks like the Build environment is having issues in the Red Hat CI tests. Will chase Monday.

if err != nil {
return err
if !strings.Contains(destSpec, "://") {
Copy link
Member

Choose a reason for hiding this comment

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

Reverse the error
if err strings.Container(destSpec,"://") {
return err
}

@TomSweeneyRedHat TomSweeneyRedHat force-pushed the dev/tsweeney/pushfix branch 3 times, most recently from d1baa9e to 7c5cfdb Compare September 15, 2017 19:50
Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
Copy link
Member Author

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

@nalind PTAL.

@rhatdan
Copy link
Member

rhatdan commented Sep 16, 2017

LGTM

@TomSweeneyRedHat
Copy link
Member Author

@nalind PTAL

@nalind
Copy link
Member

nalind commented Sep 19, 2017

LGTM

@rhatdan
Copy link
Member

rhatdan commented Sep 19, 2017

@rh-atomic-bot r+ f1b6b46

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit f1b6b46 with merge 7a2ffe5...

rh-atomic-bot pushed a commit that referenced this pull request Sep 19, 2017
Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>

Closes: #260
Approved by: rhatdan
@rh-atomic-bot
Copy link
Collaborator

💥 Test timed out

@rhatdan
Copy link
Member

rhatdan commented Sep 20, 2017

@rh-atomic-bot retest

@rhatdan
Copy link
Member

rhatdan commented Sep 21, 2017

@rh-atomic-bot retry

1 similar comment
@rhatdan
Copy link
Member

rhatdan commented Sep 21, 2017

@rh-atomic-bot retry

@rhatdan
Copy link
Member

rhatdan commented Sep 21, 2017

@rh-atomic-bot retest

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit f1b6b46 with merge 1d0b48d...

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-redhatci
Approved by: rhatdan
Pushing 1d0b48d to master...

@TomSweeneyRedHat TomSweeneyRedHat deleted the dev/tsweeney/pushfix branch September 13, 2022 02:11
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants