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

Imgadm docker #51563

Merged
merged 2 commits into from
Feb 13, 2019
Merged

Imgadm docker #51563

merged 2 commits into from
Feb 13, 2019

Conversation

sjorge
Copy link
Contributor

@sjorge sjorge commented Feb 8, 2019

What does this PR do?

After the recent issues reported with imgadm module/state on SmartOS I did a few more tests and two small improvement should be made.

  1. Up until now imgadm module assumed that docker images were always called account/image:tag, but this is not always the case, e.g. fedora:latest is a valid docker image too.

  2. auto_import would not import image for disks, this resulted in confusing errors about image_size.

What issues does this PR fix or reference?

N/a

Previous Behavior

Docker images like fedora:latest could not be imported.
Unhelpful error when disk images were missing, even with auto_import set to true.

New Behavior

Docker images like fedora:latest can now be imported. They might still not run properly, but at least now they can be imported.

With auto_import set to true we now also import disks images, this confusing error is still present with auto_import set to false, but we pass it directly from upstream (vmadm). So not much we can do about it in salt.

Tests written?

No

Commits signed with GPG?

No

@sjorge
Copy link
Contributor Author

sjorge commented Feb 9, 2019

I'm confused by the lint failures, locally they pass lint?

[hyperon :: sjorge][~/Desktop/_github/salt][imgadm_docker ✔]
[.]$ ~/.local/bin/pylint --rcfile=.testing.pylintrc --disable=W1307 salt/modules/smartos_imgadm.py
Using config file /Users/sjorge/Desktop/_github/salt/.testing.pylintrc

------------------------------------
Your code has been rated at 10.00/10

[hyperon :: sjorge][~/Desktop/_github/salt][imgadm_docker ✔]
[.]$ ~/.local/bin/pylint --rcfile=.testing.pylintrc --disable=W1307 salt/states/smartos.py
Using config file /Users/sjorge/Desktop/_github/salt/.testing.pylintrc

------------------------------------
Your code has been rated at 10.00/10

@damon-atkins
Copy link
Contributor

Just something up with the server/slave which did the lint. Just try again.

Up until now imgadm assume that docker images were always called account/image:tag,
but this is not always the case, e.g. fedora:latest is a valid docker image too.
@dwoz dwoz merged commit 04971d3 into saltstack:develop Feb 13, 2019
@sjorge sjorge deleted the imgadm_docker branch March 13, 2021 12:20
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.

3 participants