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

Master image pull fails #5024

Merged
merged 1 commit into from
Aug 8, 2017
Merged

Conversation

jkaurredhat
Copy link
Contributor

When using

openshift_use_system_containers=True

it fails as :

system_images_registry is not available in a dict

Signed-off-by: jkaurredhat jkaur@redhat.com

When using

openshift_use_system_containers=True

it fails  as :

system_images_registry is not available in a dict

Signed-off-by: jkaurredhat <jkaur@redhat.com>
@sdodson sdodson requested review from giuseppe and ashcrow August 8, 2017 17:14
@sdodson
Copy link
Member

sdodson commented Aug 8, 2017

If this isn't a backport we need to make the change on master first.

@giuseppe
Copy link
Member

giuseppe commented Aug 8, 2017

Please specify system_images_registry, we need fully specified image names for system containers. We need to document this

@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

It looks like @ingvagabund made a similar change in master in b7aea29.

@ingvagabund
Copy link
Member

ingvagabund commented Aug 8, 2017

This looks like the backport from the master of #4789.

@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

@ingvagabund @jkaurredhat where you folks not setting system_images_registry?

@ingvagabund
Copy link
Member

It needs to be set only when openshift_use_system_containers=True. The #4789 is needed to run the Ci for system containers. And yes, it needs to be backported to 3.6 as well.

Copy link
Member

@ingvagabund ingvagabund left a comment

Choose a reason for hiding this comment

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

Backport of #4789

@ingvagabund
Copy link
Member

aos-ci-test

@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

Ahh ok, it's for CI more than actual usage. In that case I'm OK with this.

Copy link
Member

@ashcrow ashcrow left a comment

Choose a reason for hiding this comment

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

Required for CI to work properly with system containers.

@ingvagabund
Copy link
Member

@jkaurredhat the docker: is here to pull the system container images in a case the images are built in docker on the same host. This should not be a valid use case in the production and we should come with a different way how to handle this use case.

@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

@ingvagabund agreed. I think as @giuseppe pointed out this is more of a lack of proper documentation letting users know they have to set the variable.

@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

FWIW I'll try to throw some minor documentation together in master to help keep this from happening in the future.

@giuseppe
Copy link
Member

giuseppe commented Aug 8, 2017

We could also set proper default values, docker hub for origin and access redhat.com for ose. What do you think?

@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

@giuseppe I'm not against that, but I think having 3 registries will become harder to maintain. As an example check the bug linked from #5031.

@openshift-bot
Copy link

error: aos-ci-jenkins/OS_3.6_NOT_containerized for fa3b164 (logs)

@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

😕

    "msg": "CA certificate /etc/origin/master/ca.crt doesn't exist on CA host 10.8.168.82. Apply 'openshift_ca' role to 10.8.168.82.\n"

@ingvagabund
Copy link
Member

@ashcrow this PR is not covered by the CI yet. So it can be safely merged. @sdodson ?

ashcrow added a commit to ashcrow/openshift-ansible that referenced this pull request Aug 8, 2017
system_images_registry is required for system container installations
but no example was put in the inventory files. This change adds a note
saying the variable is required when using system containers and
provides an example.

Ref: openshift#5024
@ashcrow
Copy link
Member

ashcrow commented Aug 8, 2017

@giuseppe
Copy link
Member

giuseppe commented Aug 8, 2017

@ashcrow in this case it will just be needed to reflect what we already do when setting the docker additional registries. Users will still be able to override it if required

@openshift-bot
Copy link

success: "aos-ci-jenkins/OS_3.6_containerized, aos-ci-jenkins/OS_3.6_containerized_e2e_tests" for fa3b164 (logs)

@sdodson sdodson merged commit 59a3eac into openshift:release-3.6 Aug 8, 2017
ashcrow added a commit to ashcrow/openshift-ansible that referenced this pull request Aug 25, 2017
system_images_registry is required for system container installations
but no example was put in the inventory files. This change adds a note
saying the variable is required when using system containers and
provides an example.

Ref: openshift#5024
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.

6 participants