Add ODG controller #411
Add ODG controller #411
Conversation
ocm-ci-robot-0
added
reviewed/ok-to-test
| import odg_operator.odg_model as odgm | ||
|
|
||
|
|
||
| ci.log.configure_default_logging() |
There was a problem hiding this comment.
I would prefer to new code to avoid dependencies to ci.* packages. esp. sth. basic like logging we should rather define locally (esp. considering that we might see a different logging infrastructure for ODG - e.g. conveying logs to some service, rather than (only) printing to stdout).
There was a problem hiding this comment.
Agree, but for logging I suggest to address this as a separate topic (there are many usages of ci.log modul across odg-extensions).
For ci.util I will copy the merge-dict function 👍
zkdev
force-pushed
the
zkdev-odg-controller
branch
3 times, most recently
from
218d87a to
484d21a
Compare
zkdev
force-pushed
the
zkdev-odg-controller
branch
5 times, most recently
from
9b20804 to
24641a8
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
zkdev
force-pushed
the
zkdev-odg-controller
branch
from
24641a8 to
dc5c0de
Compare
| @@ -0,0 +1,583 @@ | |||
| import argparse | |||
There was a problem hiding this comment.
is it, naming-wise a good match to have a package odg_operator (emphasis on the operator part), w/ a submodule odg_controller?
There was a problem hiding this comment.
An k8s operator is a method of managing a k8s application using resource, controllers, and domain specific stuff.
I prefer a dedicated repository, for the time being I think the package is okay.
There was a problem hiding this comment.
We might omit the odg_ prefix for the modules as it's already expressed by the package name odg_operator, wdyt?
There was a problem hiding this comment.
for model and util there are name collisions, thus odg prefix has to remain.
for controller we could remove it, however considering consistency and that we will restructure repositories anyways, I think we should keep the prefix for now.
There was a problem hiding this comment.
Why are there name collisions if they are part of the odg_operator package?
There was a problem hiding this comment.
well, package local import would look like that:
import utilthis would collide with top level util module
we could workaround this with "full path" (import odg_operator.util) as we typically add the repo-dir to PYTHONPATH, nevertheless I'd like to avoid introducing such pitfalls.
There was a problem hiding this comment.
Hmm, AFAIK we always import modules with "full path" within this code base, so I'd vote to use a consistent approach either way.
Edit: I just saw those imports are using the "full path" already anyways.
zkdev
force-pushed
the
zkdev-odg-controller
branch
from
dc5c0de to
7878934
Compare
| @@ -0,0 +1,583 @@ | |||
| import argparse | |||
There was a problem hiding this comment.
We might omit the odg_ prefix for the modules as it's already expressed by the package name odg_operator, wdyt?
zkdev
force-pushed
the
zkdev-odg-controller
branch
from
7878934 to
4faf902
Compare
zkdev
force-pushed
the
zkdev-odg-controller
branch
from
4faf902 to
93f3acc
Compare
zkdev
force-pushed
the
zkdev-odg-controller
branch
2 times, most recently
from
e452e0b to
acdc8c2
Compare
| @@ -0,0 +1,583 @@ | |||
| import argparse | |||
There was a problem hiding this comment.
Why are there name collisions if they are part of the odg_operator package?
Reconciles ODG resources in mODG-root cluster and creates ODG-Extension resources (via gardener-resource-manager).
zkdev
force-pushed
the
zkdev-odg-controller
branch
from
acdc8c2 to
71932bd
Compare
Reconciles ODG resources in mODG-root cluster and creates ODG-Extension resources (via gardener-resource-manager).
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note: