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 reference-policy option to oc tag #12862

Merged
merged 2 commits into from
Feb 9, 2017

Conversation

mfojtik
Copy link
Contributor

@mfojtik mfojtik commented Feb 8, 2017

No description provided.

@mfojtik mfojtik changed the title Reference policy in tag Add reference-policy option to oc tag Feb 8, 2017
@mfojtik
Copy link
Contributor Author

mfojtik commented Feb 8, 2017

@miminar can I have quick review here? seems like an easy fix for great profit :-)

I tested this:

# Set reference policy to local
$ oc tag --source=docker openshift/origin:latest test/origin:foo --reference-policy=local

# Change reference policy to source
$ oc tag --source=docker openshift/origin:latest test/origin:foo --reference-policy=source

@mfojtik
Copy link
Contributor Author

mfojtik commented Feb 8, 2017

[test]

Copy link
Contributor

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

Nit, otherwise LGTM

@@ -273,6 +278,12 @@ func (o TagOptions) Validate() error {
return errors.New("--alias and --delete may not be both specified")
}

switch o.referencePolicy {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't plain old if better suited here, this reads weird 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done :-)

@mfojtik mfojtik force-pushed the reference-policy-in-tag branch from 1c157a7 to 0d94afe Compare February 8, 2017 10:10
cmd.Flags().BoolVar(&opts.aliasTag, "alias", false, "Should the destination tag be updated whenever the source tag changes. Defaults to false.")
cmd.Flags().BoolVar(&opts.referenceTag, "reference", false, "Should the destination tag continue to pull from the source namespace. Defaults to false.")
cmd.Flags().BoolVar(&opts.scheduleTag, "scheduled", false, "Set a Docker image to be periodically imported from a remote repository. Defaults to false.")
cmd.Flags().BoolVar(&opts.insecureTag, "insecure", false, "Set to true if importing the specified Docker image requires HTTP or has a self-signed certificate. Defaults to false.")
cmd.Flags().StringVar(&opts.referencePolicy, "reference-policy", opts.referencePolicy, "Describes how pull-specs for images in the image stream tag are generated. Valid values are 'source' (default) or 'local'.")
Copy link

Choose a reason for hiding this comment

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

s/or/and/

Copy link

Choose a reason for hiding this comment

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

The help doesn't help me to understand what it does.

Copy link

Choose a reason for hiding this comment

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

What about Instruct image consumers where to pull the image from. Use 'local' to pull from integrated registry. Use 'source' to pull from the image source (default).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@miminar i don't think that describes what this does... basically this just says how the triggers should resolve the image... how about:

Specify how triggers should resolve the image. Use "local" to use integrated registry or "source" for original image source (default).

@@ -273,6 +278,10 @@ func (o TagOptions) Validate() error {
return errors.New("--alias and --delete may not be both specified")
}

if o.referencePolicy != "source" && o.referencePolicy != "local" {
Copy link

Choose a reason for hiding this comment

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

Can you use constants?

@mfojtik mfojtik force-pushed the reference-policy-in-tag branch from 0d94afe to bfdefea Compare February 8, 2017 12:48
Copy link

@miminar miminar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@mfojtik
Copy link
Contributor Author

mfojtik commented Feb 8, 2017

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to df6b2fd

@openshift-bot
Copy link
Contributor

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to df6b2fd

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13729/) (Base Commit: 863f435)

@openshift-bot
Copy link
Contributor

openshift-bot commented Feb 9, 2017

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/13729/) (Base Commit: 38d2e9b) (Image: devenv-rhel7_5882)

@openshift-bot openshift-bot merged commit 4b73eba into openshift:master Feb 9, 2017
cmd.Flags().BoolVar(&opts.aliasTag, "alias", false, "Should the destination tag be updated whenever the source tag changes. Defaults to false.")
cmd.Flags().BoolVar(&opts.referenceTag, "reference", false, "Should the destination tag continue to pull from the source namespace. Defaults to false.")
cmd.Flags().BoolVar(&opts.scheduleTag, "scheduled", false, "Set a Docker image to be periodically imported from a remote repository. Defaults to false.")
cmd.Flags().BoolVar(&opts.insecureTag, "insecure", false, "Set to true if importing the specified Docker image requires HTTP or has a self-signed certificate. Defaults to false.")
cmd.Flags().StringVar(&opts.referencePolicy, "reference-policy", sourceReferencePolicy, "Allow to request pull-trough for external image when set to 'local'. Defaults to 'source'.")
Copy link

Choose a reason for hiding this comment

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

typo in pull-trough 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hups

@miminar
Copy link

miminar commented Feb 15, 2017

Reference tagging original API change PR #12217

@mfojtik mfojtik deleted the reference-policy-in-tag branch September 5, 2018 21:08
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.

4 participants