feat(inventory): introduce OmegaConf as optional inventory backend #995

MatteoVoges · 2023-04-27T08:47:07Z

Fixes #990

Proposed Changes

introduces OmegaConf as new backend
working branches are kapicorp/nexenio-dev, neXenio/dev
see docs for all features and changes: docs/pages/inventory/omegaconf.md

To do

Contributed by

ramaro · 2023-04-27T08:52:34Z

@MatteoVoges this is very exciting, thanks! Will review in the new couple of days.

MatteoVoges · 2023-04-27T08:53:24Z

Note: the tests are failing because of the changes made in #993
Fix for that exists already in #994

gburiola · 2023-05-01T14:02:25Z

This is very exciting indeed. I haven't heard of OmegaConf before. Great find!!

reclass is great but after you use it for a while you find lots of small things that could be improved and it's a bit sad that it's not longer being maintained. So having an alternative is great.

However, reclass is everywhere in the kapitan code. So my suggestion is to open a separate PR where you just make kapitan code more generic and ready to accept a different type of inventory, without adding OmegaConf just yet. For example the changes you have in this PR to rename inventory_reclass to get_inventory. I think it's simpler to review and ensure it's backward compatible.

I believe there are lots of other places (in addition to inventory_reclass function) that needs to be adjusted.

For example:

kapitan searchvar
kapitan lint

MatteoVoges · 2023-05-03T12:28:29Z

However, reclass is everywhere in the kapitan code. So my suggestion is to open a separate PR where you just make kapitan code more generic and ready to accept a different type of inventory, without adding OmegaConf just yet. For example the changes you have in this PR to rename inventory_reclass to get_inventory. I think it's simpler to review and ensure it's backward compatible.

Yeah, I will do that. But I leave the changes in here as well, to see the full view of the change.
I don't know how we want to support several inventories. With this pr, we can toggle between OmegaConf and reclass by setting the flag --omegaconf or not. If we have several options, should each backend get their own toggle flag? Or maybe a specification flag would make sense, like --inv=MyInv or --inv=OmegaConf.

MatteoVoges · 2023-05-24T15:18:08Z

There is some error in the OmegaConf.merge() function, because it doesn't support merging of list objects. This is fixed in omry/omegaconf#1082 , but we have to wait until the next release, to use the feature

MatteoVoges · 2023-06-01T10:35:31Z

Since the list merge in Omegaconf now works, I included the module locally and was able to test it successfully. Migrating the inventory only works with ruamel.yaml at the moment, as this is the only way to preserve comments. Also the local module has to be replaced by the PyPi module again, if it is released.

If you compile the inventory this is the only change: (examples/kubernetes/compiled/minikube-nginx-helm/nginx-deploy.sh)

- KUBECTL="kubectl -n None" #(2)!
+ KUBECTL="kubectl -n minikube-nginx-helm" #(2)!

So the namespace is now correct. I think that this change was even a bug in reclass and so this is fixed now. But maybe it could be intentional...

MatteoVoges · 2023-07-05T16:59:46Z

@ademariag @ramaro
Most of the development is finished and ready to merge now. However there are tests and documentation missing. I will add them, if we agree on a final first-version of omegaconf, because then I don't have to rewrite everything 😄
I would present you the current state of the inventory in the next work group meeting with some examples.
What do you think about this?

ramaro · 2023-08-12T15:36:59Z

pyproject.toml

@@ -51,6 +51,7 @@ jsonschema = "^4.17.3"
 kadet = "^0.2.2"
 python-gnupg = "^0.4.7"
 pyyaml = "^6.0"
+omegaconf = { git = "https://github.com/neXenio/omegaconf.git", branch = "dev" }


why do we have to use dev branch?

ramaro · 2023-08-12T15:39:26Z

kapitan/resources.py

+    args = list(cached.args.values())[0]
+    use_omegaconf = args.omegaconf
+    try:
+        migrate_omegaconf = args.migrate


not really use why we need to do this inside a try?

use perhaps:

migrate_omegaconf = args.get("migrate", False)

ramaro · 2023-08-12T15:40:33Z

kapitan/resources.py

+            # inv_start = time.time()
+            inv = inventory_omegaconf(inventory_path, ignore_class_notfound, targets)
+            # logger.info("REAL_TIME (%.2fs)", time.time() - inv_start)
+        except Exception as e:
+            if not migrate_omegaconf:
+                logger.warning("Make sure to migrate your inventory using --migrate")
+            raise InventoryError(e)
+    else:
+        logger.debug("Using reclass as inventory backend")
+        # inv_start = time.time()
+        inv = inventory_reclass(inventory_path, ignore_class_notfound)
+        # logger.info("REAL_TIME (%.2fs)", time.time() - inv_start)


can we remove the commented code?

ramaro · 2023-08-12T15:46:43Z

kapitan/resolvers.py

@@ -0,0 +1,163 @@
+#!/usr/bin/env python3
+
+# Copyright 2023 nexenio


unfortunately we need to remove this. We need this instead:

# Copyright 2023 The Kapitan Authors # SPDX-FileCopyrightText: 2020 The Kapitan Authors <kapitan-admins@googlegroups.com> # # SPDX-License-Identifier: Apache-2.0

ramaro · 2023-08-12T15:47:05Z

kapitan/omegaconf_inv.py

@@ -0,0 +1,198 @@
+#!/usr/bin/env python3
+
+# Copyright 2023 nexenio


same copyright msg here too

ademariag · 2023-08-14T14:06:29Z

Makefile

+
+pull_oc:
+	rm -rf omegaconf
+	git clone --branch 1080-add-list-deep-merging https://github.com/nexenio/omegaconf.git oc


@MatteoVoges this is not needed anymore now that it has been merged right?

MatteoVoges

Hey @ramaro and @ademariag ,
This is an outdated version of the implementation. I'm now working on a nexenio/dev branch to work more independently, because some of my PR's depend on each other and we at nexenio wanted to have a kapitan branch with all of my (upcoming) features.
Of course I will continue sending parts to this upstream, but I think this will happen less frequently.

I can show you my progress on friday and then you can review it properly.

MatteoVoges added 2 commits April 26, 2023 20:39

feat(inv): introduce OmegaConf as optional inventory backend

ea3131b

remove print statement

9c57cae

MatteoVoges added feature documentation Anything related to documentation testing_needed needs some more testing to triage, or for issue solution dependencies Pull requests that update a dependency file labels Apr 27, 2023

MatteoVoges self-assigned this Apr 27, 2023

fix(lint): fix linting

770404c

Merge branch 'master' into pluggable-inventory

5de9d50

Merge branch 'master' into pluggable-inventory

c42b1b1

MatteoVoges mentioned this pull request May 3, 2023

refactor: prepare support for optional inventory backends #1003

Closed

MatteoVoges added 5 commits May 10, 2023 17:51

add first version of migration script

083160f

add flag behavior for --compose-node-name and -t

d9a1d23

add resolving step for references in migration script

fc314f7

feat: finish migration script

0e76778

feat: enable migration support as query

742b2bb

MatteoVoges marked this pull request as draft May 19, 2023 15:41

MatteoVoges added 4 commits June 1, 2023 11:44

add local module omegaconf to have new features

79bf1e9

add migrated inventory to test the feature

6b54600

refactor: add error handling and correct merging flags

60c56ea

deps: add temporarily deps ruamel and regex for migrating

b1f85e7

MatteoVoges added 3 commits June 1, 2023 16:45

feat: prepare omegaconf for multiprocessing

3b5c8db

feat: add generated grammar

d20be19

change route of module oc

a7944a6

MatteoVoges added 3 commits June 22, 2023 15:19

feat: add more custom resolvers

be32b38

refactor: add more resolvers

641dc73

fix: namespace error with flag migrate

7dc7585

MatteoVoges force-pushed the pluggable-inventory branch from d68cc97 to 7dc7585 Compare June 29, 2023 09:32

MatteoVoges added 5 commits June 29, 2023 12:05

feat: add ability to define user resolvers in inventory

47b2c54

fix: user written resolvers replace system resolvers

b605005

feat: restructure resolving and migrating

3c616d5

chore: remove ruamel-yaml and add omegaconf in poetry-file

08a2194

Merge branch 'master' into pluggable-inventory

228ac23

MatteoVoges marked this pull request as ready for review July 5, 2023 16:48

MatteoVoges requested review from ramaro and ademariag July 5, 2023 16:48

MatteoVoges added 4 commits July 12, 2023 10:23

fix: resolver escape_tag was missing braces

ba04981

fix: correct wrong behavior of resolver tag

78e83ff

feat: prepare support for lint

1f7923a

Merge branch 'dev' into pluggable-inventory

8fd6d93

ramaro reviewed Aug 12, 2023

View reviewed changes

kapitan/omegaconf_inv.py

@@ -0,0 +1,198 @@

#!/usr/bin/env python3

# Copyright 2023 nexenio

Copy link

Member

ramaro Aug 12, 2023

Choose a reason for hiding this comment

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

same copyright msg here too

ademariag reviewed Aug 14, 2023

View reviewed changes

MatteoVoges commented Aug 15, 2023

View reviewed changes

MatteoVoges marked this pull request as draft September 1, 2023 15:15

MatteoVoges closed this Sep 19, 2023

MatteoVoges deleted the pluggable-inventory branch October 30, 2023 17:07

MatteoVoges restored the pluggable-inventory branch January 17, 2024 08:45

MatteoVoges reopened this Jan 17, 2024

MatteoVoges closed this Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(inventory): introduce OmegaConf as optional inventory backend #995

feat(inventory): introduce OmegaConf as optional inventory backend #995

MatteoVoges commented Apr 27, 2023 •

edited

Loading

ramaro commented Apr 27, 2023

MatteoVoges commented Apr 27, 2023

gburiola commented May 1, 2023

MatteoVoges commented May 3, 2023 •

edited

Loading

MatteoVoges commented May 24, 2023

MatteoVoges commented Jun 1, 2023

MatteoVoges commented Jul 5, 2023

ramaro Aug 12, 2023

ramaro Aug 12, 2023

ademariag Aug 14, 2023

ramaro Aug 12, 2023

ramaro Aug 12, 2023

ramaro Aug 12, 2023

ademariag Aug 14, 2023

MatteoVoges left a comment

		@@ -0,0 +1,163 @@
		#!/usr/bin/env python3

		# Copyright 2023 nexenio

		@@ -0,0 +1,198 @@
		#!/usr/bin/env python3

		# Copyright 2023 nexenio

feat(inventory): introduce OmegaConf as optional inventory backend #995

feat(inventory): introduce OmegaConf as optional inventory backend #995

Conversation

MatteoVoges commented Apr 27, 2023 • edited Loading

Proposed Changes

To do

Contributed by

ramaro commented Apr 27, 2023

MatteoVoges commented Apr 27, 2023

gburiola commented May 1, 2023

MatteoVoges commented May 3, 2023 • edited Loading

MatteoVoges commented May 24, 2023

MatteoVoges commented Jun 1, 2023

MatteoVoges commented Jul 5, 2023

ramaro Aug 12, 2023

Choose a reason for hiding this comment

ramaro Aug 12, 2023

Choose a reason for hiding this comment

ademariag Aug 14, 2023

Choose a reason for hiding this comment

ramaro Aug 12, 2023

Choose a reason for hiding this comment

ramaro Aug 12, 2023

Choose a reason for hiding this comment

ramaro Aug 12, 2023

Choose a reason for hiding this comment

ademariag Aug 14, 2023

Choose a reason for hiding this comment

MatteoVoges left a comment

Choose a reason for hiding this comment

MatteoVoges commented Apr 27, 2023 •

edited

Loading

MatteoVoges commented May 3, 2023 •

edited

Loading