Skip to content

qa/cephadm: Docker doesn't ship a registries.conf#35273

Merged
sebastian-philipp merged 1 commit intoceph:masterfrom
sebastian-philipp:qa-cephadm-mirror-ubuntu
May 27, 2020
Merged

qa/cephadm: Docker doesn't ship a registries.conf#35273
sebastian-philipp merged 1 commit intoceph:masterfrom
sebastian-philipp:qa-cephadm-mirror-ubuntu

Conversation

@sebastian-philipp
Copy link
Contributor

I guess not using the mirror for docker based tests is ok for now.

For adding docker support, we need to:

  1. change the docker config
  2. restart the docker daemon

Note that Docker's config only supports to mirror the docker.io registry.

Signed-off-by: Sebastian Wagner sebastian.wagner@suse.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

I guess not using the mirror for docker based tests is ok for now.

For adding docker support, we need to:

1. change the docker config
2. restart the docker daemon

Note that Docker's config only supports to mirror the docker.io registry.

Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
@varshar16
Copy link
Contributor

jenkins retest this please

)
except FileNotFoundError as e:
# Docker doesn't ship a registries.conf
log.info('Failed to add mirror: %s' % str(e))
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 it an error and we should stop testing now?

Copy link
Contributor Author

@sebastian-philipp sebastian-philipp May 27, 2020

Choose a reason for hiding this comment

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

if the file isn't there, we can simply fall back to the default config and download from docker.io

worst case is: the ubuntu based tests fail. And at that point, we have to also look into the things I stated in the PR description

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand you guys are in a rush that you merged it. But since you were trying to override the non-existing file, it is definitely an error, at least, you might want to use log.warning... ;-) otherwise I don't see a reason to inform user that is missing anyways.
PS: The PR description does not say much to me.

@tchaikov tchaikov self-assigned this May 27, 2020
@sebastian-philipp
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants