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

Implement override and reset analog to docker-compose #830

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 82 additions & 2 deletions podman_compose.py
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,47 @@ def flat_deps(services, with_extends=False):
for name, srv in services.items():
rec_deps(services, name)

###################
# Override and reset tags
###################

class OverrideTag(yaml.YAMLObject):
yaml_dumper = yaml.Dumper
yaml_loader = yaml.SafeLoader
yaml_tag = u'!override'
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I remember correctly, u prefix is from python 2 times. Am I correct that it's not needed here?


def __init__(self, value):
values = list()

for item in value:
values.append(item.value)

self.value = values
Copy link
Collaborator

Choose a reason for hiding this comment

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

self.value = [item.value for item in value]


@classmethod
def from_yaml(cls, loader, node):
return OverrideTag(node.value)

@classmethod
def to_yaml(cls, dumper, data):
return dumper.represent_scalar(cls.yaml_tag, data.value)

class ResetTag(yaml.YAMLObject):
yaml_dumper = yaml.Dumper
yaml_loader = yaml.SafeLoader
yaml_tag = u'!reset'

@classmethod
def toJSON(self):
return self.yaml_tag

@classmethod
def from_yaml(cls, loader, node):
return ResetTag()

@classmethod
def to_yaml(cls, dumper, data):
return dumper.represent_scalar(cls.yaml_tag, '')

###################
# podman and compose classes
Expand Down Expand Up @@ -1245,6 +1286,14 @@ def volume_ls(self, proj=None):


def normalize_service(service, sub_dir=""):
if isinstance(service, ResetTag):
return service

if isinstance(service, OverrideTag):
tagObj = service
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use tag_obj spelling.

service = tagObj.value
print(subdir)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unneeded print.


if "build" in service:
build = service["build"]
if is_str(build):
Expand Down Expand Up @@ -1284,7 +1333,12 @@ def normalize_service(service, sub_dir=""):
if is_str(extends):
extends = {"service": extends}
service["extends"] = extends
return service

try:
tagObj.value = serivce
Copy link

Choose a reason for hiding this comment

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

Is this a typo?

Copy link
Author

Choose a reason for hiding this comment

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

No you need to still normalize the values of the Override-tag to be valid in the end (as they are the ones used) but pass the override-tag through to the recursive merge to do the actual override. I used try to determinate if there is a tagObj defiend and if so get the normalized values to the tagObj and return it otherwise return the normalized service-definition.

Copy link
Author

Choose a reason for hiding this comment

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

Also this isn't my most recent version anymore as I'm locally working an a version based on the latest https://github.com/maurerle/podman-compose but for now using docker-compose again.

Copy link

Choose a reason for hiding this comment

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

Thanks for the explanation 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

serivce seems to be non-existing variable though.

Copy link
Author

Choose a reason for hiding this comment

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

Edit: You are all right - missed the typo but that's not an issue/the latest state and needs a rebase.

How likely are my chances to get this approach merged, if I resubmit it?

Copy link
Collaborator

@p12tic p12tic Mar 8, 2024

Choose a reason for hiding this comment

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

Good question, haven't looked into viability of the PR so far. Could you point me to the part of the compose spec that this implements? I didn't find with couple minutes of googling and better I don't make an error here :) Whoops, it's in PR description

Overall I think podman-compose should support all things possible in non-swarm compose that can be implemented on podman. We may argue about the approach of implementation though.

What I will definitely ask that's not part of the PR right now is unit and integration tests.

return tagObj
except NameError:
return service


def normalize(compose):
Expand Down Expand Up @@ -1330,6 +1384,8 @@ def rec_merge_one(target, source):
update target from source recursively
"""
done = set()
remove = set()

for key, value in source.items():
if key in target:
continue
Expand All @@ -1339,17 +1395,37 @@ def rec_merge_one(target, source):
if key in done:
continue
if key not in source:
if isinstance(value, ResetTag):
log("INFO: Unneeded !reset found for [{key}]")
remove.add(key)

if isinstance(value, OverrideTag):
log("INFO: Unneeded !override found for [{key}] with value '{value}'")
target[key] = clone(value.value)

continue

value2 = source[key]

if isinstance(value, ResetTag) or isinstance(value2, ResetTag):
remove.add(key)
continue

if isinstance(value, OverrideTag) or isinstance(value2, OverrideTag):
target[key] = clone(value.value) if isinstance(value, OverrideTag) else clone(value2.value)
continue

if key in ("command", "entrypoint"):
target[key] = clone(value2)
continue

if not isinstance(value2, type(value)):
value_type = type(value)
value2_type = type(value2)
raise ValueError(
f"can't merge value of [{key}] of type {value_type} and {value2_type}"
)

if is_list(value2):
if key == "volumes":
# clean duplicate mount targets
Expand All @@ -1368,6 +1444,10 @@ def rec_merge_one(target, source):
rec_merge_one(value, value2)
else:
target[key] = value2

for key in remove:
del target[key]

return target


Expand Down Expand Up @@ -1629,7 +1709,7 @@ def _parse_compose_file(self):
compose["services"] = resolved_services
if not getattr(args, "no_normalize", None):
compose = normalize_final(compose, self.dirname)
self.merged_yaml = yaml.safe_dump(compose)
self.merged_yaml = yaml.dump(compose)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this strictly needed?

merged_json_b = json.dumps(compose, separators=(",", ":")).encode("utf-8")
self.yaml_hash = hashlib.sha256(merged_json_b).hexdigest()
compose["_dirname"] = dirname
Expand Down