Split the populator image

[liranr23]

This patch will split the populator image.
One image acts as a controller to the populator - 'populator-controller' and the actual populator action will be executed by the 'ovirt-populator' image.

This will allow a separation in logic, allowing to add more source providers images and use the same controller for all.

Signed-off-by: Liran Rotenberg [email protected]

[liranr23]

Building the ovirt-populator image works. Locally it also runs, but on my OCP it doesn't - exec /ovirt-populator: exec format error.
There are some bugs that seems to relate the issue I see golang/go#53116 and golang/go#53804 (Basing on golang/go#53804 (comment), it relates to CGO_ENABLED=0. There is also a kernel fix in v6.1 and above. But the images used to compile is using 5.14.0-162.6.1.el9_1.x86_64. Still, it does seem to work locally.

[liranr23]

Building the ovirt-populator image works. Locally it also runs, but on my OCP it doesn't - exec /ovirt-populator: exec format error. There are some bugs that seems to relate the issue I see golang/go#53116 and golang/go#53804 (Basing on golang/go#53804 (comment), it relates to CGO_ENABLED=0. There is also a kernel fix in v6.1 and above. But the images used to compile is using 5.14.0-162.6.1.el9_1.x86_64. Still, it does seem to work locally.

works now (somehow, my guess it's either luck or changing it into cmd directory) :)

[codecov]

Codecov Report

Base: 33.89% // Head: 33.91% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (0e78067) compared to base (bc83af6).
Patch has no changes to coverable lines.

❗ Current head 0e78067 differs from pull request most recent head 0c200aa. Consider uploading reports for the commit 0c200aa to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
+ Coverage   33.89%   33.91%   +0.02%     
==========================================
  Files          42       42              
  Lines        8098     8093       -5     
==========================================
  Hits         2745     2745              
+ Misses       5136     5131       -5     
  Partials      217      217              

Flag	Coverage Δ
unittests	33.91% <ø> (+0.02%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...m/kubev2v/forklift/pkg/controller/plan/kubevirt.go	2.04% <0.00%> (+<0.01%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[bennyz]

Right I forgot we have the group and kind

…

On Sunday, February 5, 2023, Liran Rotenberg ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In cmd/populator-controller/populator-controller.go <#135 (comment)>: > + flag.StringVar(&kubeconfig, "kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.") + } + flag.StringVar(&masterURL, "master", "", "The address of the Kubernetes API server. Overrides any value in kubeconfig. Only required if out-of-cluster.") + flag.StringVar(&imageName, "image-name", "", "Image to use for populating") + // Metrics args + flag.StringVar(&httpEndpoint, "http-endpoint", "", "The TCP network address where the HTTP server for diagnostics, including metrics and leader election health check, will listen (example: `:8080`). The default is empty string, which means the server is disabled.") + flag.StringVar(&metricsPath, "metrics-path", "/metrics", "The HTTP path where prometheus metrics will be exposed. Default is `/metrics`.") + // Other args + flag.StringVar(&namespace, "namespace", "konveyor-forklift", "Namespace to deploy controller") + flag.Parse() + + var ( + gk = schema.GroupKind{Group: groupName, Kind: kind} + gvr = schema.GroupVersionResource{Group: groupName, Version: apiVersion, Resource: resource} + ) + imageName = "quay.io/lrotenbe/ovirt-populator:test" if it's in the CR, we can use the populator-controller image to take it in provide it to the the library, without modifying the library further. — Reply to this email directly, view it on GitHub <#135 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQKN4UGH5OTXYIXK7YMMVLWV5QEBANCNFSM6AAAAAAUMU4FG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[liranr23]

Right I forgot we have the group and kind

if it's in the CR, we can use the populator-controller image to take it in provide it to the the library, without modifying the library further.

So, I think we don't really need to change a thing? once we have other kinds we may change the image based on that? or we should providing an image anyway in the CR?

[ahadas]

@liranr23 you mean another provider type that uses volume-populator? if so I don't see why not to do it before - even if ovirt is the only provider, I'd still expect the image of the populator to be configurable (and propagated from the controller that takes the forklift or mtv image that is set in the operator)

[liranr23]

@liranr23 you mean another provider type that uses volume-populator? if so I don't see why not to do it before - even if ovirt is the only provider, I'd still expect the image of the populator to be configurable (and propagated from the controller that takes the forklift or mtv image that is set in the operator)

benny and i discussed offline, we think a configmap might be a solution for this. i'm still bothered with how the controller will know which kind he will be looking for.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
6.5% Duplication

[ahadas]

@liranr23 please check the failure in:
https://github.com/kubev2v/forklift/actions/runs/4117193517/jobs/7108193103 (and @mnecas please review)