-
Notifications
You must be signed in to change notification settings - Fork 30
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
Allow to set insecure registry with minishift #52
Conversation
It should work but I'll test with our pipeline and then add comment here. |
LGTM. |
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.
LGTM
@@ -86,6 +86,7 @@ contra-env-setup/playbooks/group_vars/all/global.yml | |||
* memory: Memory size to use for the VM : default=6400mb | |||
* cpus: Number of cpus to use for minishift VM: default=2 | |||
* minishift_iso: ISO image to use : default=http://artifacts.ci.centos.org/fedora-atomic/minishift/iso/minishift.iso | |||
* minishift_insecure_registry: Additional insecure registries : default="" (not used) |
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.
Should we call this minishift_insecure_registries
since it can contain multiple registries?
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.
I'm fine even with minishift_insecure_registries
I used singular because of singular in minishift config.
minishift config set insecure-registry
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.
OK, I wasn't aware of that. Let's keep the singular then.
@@ -28,6 +28,10 @@ | |||
- name: "Set minishift openshift-version to {{ oc_version }}" | |||
shell: "{{ minishift_bin }} config set openshift-version {{ oc_version }}" | |||
|
|||
- name: "Set minishift minishift_insecure_registry" | |||
shell: "{{ minishift_bin }} config set insecure-registry \"{{ minishift_insecure_registry }}\"" |
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 take a list?
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.
yes
[root@host~]# minishift config view
- cpus : 2
- disk-size : 40gb
- memory : 6400mb
- openshift-version : v3.9.0
- skip-check-openshift-release : true
- skip-check-openshift-version : true
[root@host ~]# minishift config set insecure-registry registry1.example.com
[root@host ~]# minishift config view
- cpus : 2
- disk-size : 40gb
- insecure-registry : [registry1.example.com]
- memory : 6400mb
- openshift-version : v3.9.0
- skip-check-openshift-release : true
- skip-check-openshift-version : true
[root@host ~]# minishift config set insecure-registry registry1.example.com,registry2.example.com
[root@host ~]# minishift config view
- cpus : 2
- disk-size : 40gb
- insecure-registry : [registry1.example.com registry2.example.com]
- memory : 6400mb
- openshift-version : v3.9.0
- skip-check-openshift-release : true
- skip-check-openshift-version : true
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.
I think names should match to what minishift uses so should be singular IMHO
No description provided.