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

Add helper image merger to merge yaml files #428

Merged
merged 8 commits into from
Aug 6, 2021
Merged

Add helper image merger to merge yaml files #428

merged 8 commits into from
Aug 6, 2021

Conversation

lukaszo
Copy link
Contributor

@lukaszo lukaszo commented Aug 5, 2021

Description

Changes proposed in this pull request:

  • Add helper image merger to merge yaml files

Testing

Build image

docker build -t merger .

Create few yaml files:

input.yaml:

a: 1

additional:yaml:

b: 2

extra.yaml:

c: 3

Run merger

docker run --mount src=`pwd`,target=/yamls,type=bind -e OUT=/yamls/all.yaml merger

Check the output file:
all.yaml

input:
  a: 1
additional:
  b: 2
extra:
  c: 3

Related issue(s)

#419

It can be used to merge differnet yaml files before passing them to
jinja2 template
Copy link
Member

@mszostok mszostok left a comment

Choose a reason for hiding this comment

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

Tested and works good 👍 but please address small comments

hack/images/merger/README.md Outdated Show resolved Hide resolved
hack/images/merger/README.md Outdated Show resolved Hide resolved
hack/images/merger/README.md Outdated Show resolved Hide resolved
hack/images/merger/README.md Outdated Show resolved Hide resolved
hack/images/merger/merger.sh Outdated Show resolved Hide resolved
@mszostok mszostok added area/hub-manifests Relates to Hub manifests enhancement New feature or request labels Aug 5, 2021
lukaszo and others added 5 commits August 5, 2021 18:04
Co-authored-by: Mateusz Szostok <szostok.mateusz@gmail.com>
Co-authored-by: Mateusz Szostok <szostok.mateusz@gmail.com>
Co-authored-by: Mateusz Szostok <szostok.mateusz@gmail.com>
Co-authored-by: Mateusz Szostok <szostok.mateusz@gmail.com>
@@ -13,5 +13,6 @@ The structure of the folder looks as follows:
```
├── jinja2 # Image containing a containerized jinja CLI tool assisting with rendering templatized OCF content
├── json-go-gen # The image that contains quicktype to generates Go struct from JSON Schemas
└── graphql-schema-linter # The image that contains graphql-schema-linter to lint GraphQL files
├── graphql-schema-linter # The image that contains graphql-schema-linter to lint GraphQL files
└── merger # The image that contains yaml merger helper scriptyaml merger helper script
Copy link
Member

@mszostok mszostok Aug 6, 2021

Choose a reason for hiding this comment

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

Suggested change
└── merger # The image that contains yaml merger helper scriptyaml merger helper script
└── merger # The image that contains YAML merger helper

hm.. sth was merged here 🤔 😜

NOTE:
I also thought about not changing the input YAML files, or maybe put them in /yamls/processed/*. Now when you executed the docker run.. twice then it will generate different output. So maybe in the future if we will implement argo retry for failed step it may cause some not deterministic output. On the other hand, we have a copy of artifacts, so maybe it will work properly. For now it can stay as it is 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I see, during argo retry the pod is recreated(with the same name) and artifacts are downloaded again. We should be good here

Co-authored-by: Mateusz Szostok <szostok.mateusz@gmail.com>
@lukaszo lukaszo merged commit 5a05e5c into capactio:main Aug 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/hub-manifests Relates to Hub manifests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants