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

Support insecure registry and enhance parsing of image stream tag #547

Merged
merged 1 commit into from
Apr 7, 2017

Conversation

qujinping
Copy link
Contributor

@qujinping qujinping commented Apr 6, 2017

Add 2 enhancements for openshift:
- support insecure docker registry for generation of openshift image stream
we add an import policy for the image stream tag by following instruction of the command argument
- enhance parsing of image tag
current implementation doesn't support format like "myregistryhost:5000/fedora/httpd:version1.0". Enhance the parsing logic.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 6, 2017
@qujinping
Copy link
Contributor Author

qujinping commented Apr 6, 2017

@cdrage @kadel ,

Please kindly review this PR again. Very sorry for the inconvenience.

I close the #506 because I messed things up while merging from upstream.

Copy link
Member

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

Other than the descriptions, code LGTM!

cmd/convert.go Outdated
@@ -101,9 +103,11 @@ func init() {

// OpenShift only
convertCmd.Flags().BoolVar(&ConvertDeploymentConfig, "deployment-config", true, "Generate an OpenShift deploymentconfig object")
convertCmd.Flags().BoolVar(&ConvertInsecureRepo, "insecure-repository", false, "Specify to use insecure docker repository while generating Openshift image stream object")
Copy link
Member

Choose a reason for hiding this comment

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

Hi @qujinping this description may be better:
Use an insecure Docker repository for OpenShift ImageStream

cmd/up.go Outdated
@@ -56,5 +58,6 @@ var upCmd = &cobra.Command{
func init() {
upCmd.Flags().BoolVar(&UpEmptyVols, "emptyvols", false, "Use empty volumes. Do not generate PersistentVolumeClaim")
upCmd.Flags().IntVar(&UpReplicas, "replicas", 1, "Specify the number of replicas generated")
upCmd.Flags().BoolVar(&UpInsecureRepo, "insecure-repository", false, "Specify to use insecure docker repository while generating Openshift image stream object")
Copy link
Member

Choose a reason for hiding this comment

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

Hi @qujinping this description may be better:
Use an insecure Docker repository for OpenShift ImageStream

@cdrage
Copy link
Member

cdrage commented Apr 6, 2017

@qujinping Saw the changes!

One last thing and we can merge! Can you please squash your commits?

@kadel
Copy link
Member

kadel commented Apr 6, 2017

One last thing and we can merge! Can you please squash your commits?

yep, this is last think even for me. Once this get squashed its ready to be merged.

@qujinping
Copy link
Contributor Author

Squashed as it should be :)

Thanks.

@cdrage
Copy link
Member

cdrage commented Apr 7, 2017

LGTM! Thanks a lot for your contributions @qujinping!!

@cdrage cdrage merged commit e8b98e3 into kubernetes:master Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants