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.

Sign up for GitHub

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

Jump to bottom

[ProcessContainers] Update Dobby implementation #1771

Merged
merged 7 commits into from
Oct 15, 2024
Merged

[ProcessContainers] Update Dobby implementation #1771

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
accfe34
[ProcessContainers] Update Dobby implementation
sebaszm Oct 8, 2024
163ce57
Merge branch 'master' into development/dobby-sync
MFransen69 Oct 10, 2024
c6b16ef
Remove commented out InternalLock calls
sebaszm Oct 10, 2024
d98ed83
Remove notice
sebaszm Oct 11, 2024
c322740
Merge branch 'master' into development/dobby-sync
sebaszm Oct 11, 2024
91f6998
Merge branch 'master' into development/dobby-sync
MFransen69 Oct 14, 2024
00bc00e
Merge branch 'master' into development/dobby-sync
MFransen69 Oct 15, 2024
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
73 changes: 32 additions & 41 deletions .../extensions/processcontainers/implementations/DobbyImplementation/DobbyImplementation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
* limitations under the License.
*/


#include "processcontainers/Messaging.h"
#include "processcontainers/ContainerProducer.h"
#include "processcontainers/ContainerAdministrator.h"
#include "processcontainers/common/CGroupContainerInfo.h"

#include "DobbyImplementation.h"
#include <Dobby/DobbyProxy.h>
#include <Dobby/IpcService/IpcFactory.h>
Expand All @@ -26,31 +32,22 @@
namespace Thunder {

namespace ProcessContainers {
// Container administrator
// ----------------------------------
IContainerAdministrator& ProcessContainers::IContainerAdministrator::Instance()
{
static DobbyContainerAdministrator& dobbyContainerAdministrator = Core::SingletonType<DobbyContainerAdministrator>::Instance();

return dobbyContainerAdministrator;
}

IContainer* DobbyContainerAdministrator::Container(const string& id, IStringIterator& searchpaths, const string& logpath, const string& configuration VARIABLE_IS_NOT_USED)
Core::ProxyType<IContainer> DobbyContainerAdministrator::Container(const string& id, IStringIterator& searchpaths,
const string& logpath, const string& configuration VARIABLE_IS_NOT_USED)
{
searchpaths.Reset(0);
while (searchpaths.Next()) {
auto path = searchpaths.Current();

auto createContainer = [&](bool useSpecFile, const string &path) -> IContainer* {
auto createContainer = [&](bool useSpecFile, const string &path) -> Core::ProxyType<IContainer> {
// Make sure no leftover will interfere...
if (ContainerNameTaken(id)) {
DestroyContainer(id);
}
this->InternalLock();
DobbyContainer* container = new DobbyContainer(id, path, logpath, useSpecFile);
InsertContainer(container);
this->InternalUnlock();

Core::ProxyType<IContainer> container;
container = ContainerAdministrator::Instance().Create<DobbyContainer>(*this, id, path, logpath, useSpecFile);
return container;
};

Expand All @@ -71,17 +68,16 @@ namespace ProcessContainers {

TRACE(Trace::Error, (_T("Could not find suitable container config for %s in any search path"), id.c_str()));

return nullptr;
return Core::ProxyType<IContainer>();
}

DobbyContainerAdministrator::DobbyContainerAdministrator()
: BaseContainerAdministrator()
uint32_t DobbyContainerAdministrator::Initialize(const string& config VARIABLE_IS_NOT_USED)
{
mIpcService = AI_IPC::createIpcService("unix:path=/var/run/dbus/system_bus_socket", "org.rdk.dobby.processcontainers");

if (!mIpcService) {
TRACE(Trace::Error, (_T("Failed to create Dobby IPC service")));
return;
return Core::ERROR_GENERAL;
} else {
// Start the IPCService which kicks off the event dispatcher thread
mIpcService->start();
Expand All @@ -90,15 +86,15 @@ namespace ProcessContainers {
// Create a DobbyProxy remote service that wraps up the dbus API
// calls to the Dobby daemon
mDobbyProxy = std::make_shared<DobbyProxy>(mIpcService, DOBBY_SERVICE, DOBBY_OBJECT);
}

DobbyContainerAdministrator::~DobbyContainerAdministrator()
{
return Core::ERROR_NONE;
}

void DobbyContainerAdministrator::Logging(const string& logPath VARIABLE_IS_NOT_USED, const string& loggingOptions VARIABLE_IS_NOT_USED)
void DobbyContainerAdministrator::Deinitialize()
{
// Only container-scope logging
mDobbyProxy.reset();
mIpcService->stop();
mIpcService.reset();
}

void DobbyContainerAdministrator::DestroyContainer(const string& name)
Expand All @@ -115,7 +111,7 @@ namespace ProcessContainers {

// Dobby stop is async - block until we get the notification the container
// has actually stopped
this->InternalLock();
// this->InternalLock();
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please remove this line if it's not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

InternalLock() was used to protect the internal list of created containers, which is now handled outside of container provider implementations (since there can be multiple providers). Therefore the InternalLock() method has been removed altogether.

In this context there container list was not modified, so using this lock was not required. But, perhaps, it was reused for a different purpose, so it was only commented out - as a remark, because it may need reviewing whether a critical section is indeed needed here.

I will remove this, since the original purpose of InternalLock is gone.


_stopPromise = std::promise<void>();
const void* vp = static_cast<void*>(new std::string(name));
Expand All @@ -139,7 +135,7 @@ namespace ProcessContainers {
}
}

this->InternalUnlock();
// this->InternalUnlock();
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this line as well


// Always make sure we unregister our callback
mDobbyProxy->unregisterListener(listenerId);
Expand Down Expand Up @@ -186,8 +182,9 @@ namespace ProcessContainers {

// Container
// ------------------------------------
DobbyContainer::DobbyContainer(const string& name, const string& path, const string& logPath, bool useSpecFile)
DobbyContainer::DobbyContainer(DobbyContainerAdministrator& admin, const string& name, const string& path, const string& logPath, const bool useSpecFile)
: _adminLock()
, _admin(admin)
, _name(name)
, _path(path)
, _logPath(logPath)
Expand All @@ -199,13 +196,9 @@ namespace ProcessContainers {

DobbyContainer::~DobbyContainer()
{
auto& admin = static_cast<DobbyContainerAdministrator&>(DobbyContainerAdministrator::Instance());

if (admin.ContainerNameTaken(_name) == true) {
if (_admin.ContainerNameTaken(_name) == true) {
Stop(Core::infinite);
}

admin.RemoveContainer(this);
}

const string& DobbyContainer::Id() const
Expand All @@ -216,10 +209,9 @@ namespace ProcessContainers {
uint32_t DobbyContainer::Pid() const
{
uint32_t returnedPid = 0;
auto& admin = static_cast<DobbyContainerAdministrator&>(DobbyContainerAdministrator::Instance());

if (_pid.IsSet() == false) {
std::string containerInfoString = admin.mDobbyProxy->getContainerInfo(_descriptor);
std::string containerInfoString = _admin.mDobbyProxy->getContainerInfo(_descriptor);

if (containerInfoString.empty()) {
TRACE(Trace::Warning, (_T("Failed to get info for container %s"), _name.c_str()));
Expand Down Expand Up @@ -315,10 +307,9 @@ namespace ProcessContainers {
bool DobbyContainer::IsRunning() const
{
bool result = false;
auto& admin = static_cast<DobbyContainerAdministrator&>(DobbyContainerAdministrator::Instance());

// We got a state back successfully, work out what that means in English
switch (static_cast<IDobbyProxyEvents::ContainerState>(admin.mDobbyProxy->getContainerState(_descriptor))) {
switch (static_cast<IDobbyProxyEvents::ContainerState>(_admin.mDobbyProxy->getContainerState(_descriptor))) {
case IDobbyProxyEvents::ContainerState::Invalid:
result = false;
break;
Expand Down Expand Up @@ -350,8 +341,6 @@ namespace ProcessContainers {
bool result = false;

_adminLock.Lock();
auto& admin = static_cast<DobbyContainerAdministrator&>(DobbyContainerAdministrator::Instance());

std::list<int> emptyList;

// construct the full command to run with all the arguments
Expand All @@ -372,9 +361,9 @@ namespace ProcessContainers {
std::string containerSpecString = specFileStream.str();
TRACE(ProcessContainers::ProcessContainerization, (_T("container spec string: %s"), containerSpecString.c_str()));

_descriptor = admin.mDobbyProxy->startContainerFromSpec(_name, containerSpecString , emptyList, fullCommand);
_descriptor = _admin.mDobbyProxy->startContainerFromSpec(_name, containerSpecString , emptyList, fullCommand);
} else {
_descriptor = admin.mDobbyProxy->startContainerFromBundle(_name, _path, emptyList, fullCommand);
_descriptor = _admin.mDobbyProxy->startContainerFromBundle(_name, _path, emptyList, fullCommand);
}
// startContainer returns -1 on failure
if (_descriptor <= 0) {
Expand All @@ -395,9 +384,8 @@ namespace ProcessContainers {
bool result = false;

_adminLock.Lock();
auto& admin = static_cast<DobbyContainerAdministrator&>(DobbyContainerAdministrator::Instance());

bool stoppedSuccessfully = admin.mDobbyProxy->stopContainer(_descriptor, false);
bool stoppedSuccessfully = _admin.mDobbyProxy->stopContainer(_descriptor, false);

if (!stoppedSuccessfully) {
TRACE(Trace::Error, (_T("Failed to stop container, internal Dobby error. id: %s descriptor: %d"), _name.c_str(), _descriptor));
Expand All @@ -409,6 +397,9 @@ namespace ProcessContainers {
return result;
}

// FACTORY REGISTRATION
static ContainerProducerRegistrationType<DobbyContainerAdministrator, IContainer::containertype::DOBBY> registration;

} // namespace ProcessContainers

} // namespace Thunder
44 changes: 19 additions & 25 deletions ...ce/extensions/processcontainers/implementations/DobbyImplementation/DobbyImplementation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,9 @@

#pragma once

#include "processcontainers/ProcessContainer.h"
#include "processcontainers/common/BaseAdministrator.h"
#include "processcontainers/common/BaseRefCount.h"
#include "processcontainers/ContainerAdministrator.h"
#include "processcontainers/common/CGroupContainerInfo.h"

#include "Messaging.h"

#include <Dobby/DobbyProtocol.h>
#include <Dobby/Public/Dobby/IDobbyProxy.h>

Expand All @@ -37,30 +33,30 @@ namespace ProcessContainers {
const string CONFIG_NAME = "/config.json";
const string CONFIG_NAME_SPEC = "/spec.json";

class DobbyContainer : public BaseRefCount<IContainer> {
private:
friend class DobbyContainerAdministrator;

DobbyContainer(const string& name, const string& path, const string& logPath, bool useSpecFile = false);
class DobbyContainerAdministrator;

class DobbyContainer : public IContainer {
public:
DobbyContainer(DobbyContainerAdministrator& admin, const string& name, const string& path, const string& logPath, const bool useSpecFile = false);
DobbyContainer(const DobbyContainer&) = delete;
DobbyContainer& operator=(const DobbyContainer&) = delete;
~DobbyContainer() override;

public:
// IContainerMethods
containertype Type() const override { return IContainer::DOBBY; }
const string& Id() const override;
uint32_t Pid() const override;
bool IsRunning() const override;
bool Start(const string& command, IStringIterator& parameters) override;
bool Stop(const uint32_t timeout /*ms*/) override;

IMemoryInfo* Memory() const override;
IProcessorInfo* ProcessorInfo() const override;
INetworkInterfaceIterator* NetworkInterfaces() const override;

private:
mutable Core::CriticalSection _adminLock;
DobbyContainerAdministrator& _admin;
string _name;
string _path;
string _logPath;
Expand All @@ -69,29 +65,27 @@ namespace ProcessContainers {
bool _useSpecFile;
};

class DobbyContainerAdministrator : public BaseContainerAdministrator<DobbyContainer> {
private:
class DobbyContainerAdministrator : public IContainerProducer {
friend class DobbyContainer;
friend class Core::SingletonType<DobbyContainerAdministrator>;

DobbyContainerAdministrator();
public:
DobbyContainerAdministrator() = default;
~DobbyContainerAdministrator() override = default;
DobbyContainerAdministrator(const DobbyContainerAdministrator&) = delete;
DobbyContainerAdministrator& operator=(const DobbyContainerAdministrator&) = delete;

private:
std::shared_ptr<AI_IPC::IIpcService> mIpcService; // Ipc Service instance
std::shared_ptr<IDobbyProxy> mDobbyProxy; // DobbyProxy instance
public:
~DobbyContainerAdministrator() override;

IContainer* Container(const string& id,
IStringIterator& searchpaths,
const string& logpath,
const string& configuration) override; //searchpaths will be searched in order in which they are iterated

// IContainerAdministrator methods
void Logging(const string& logDir, const string& loggingOptions) override;
public:
// IContainerProducer methods
uint32_t Initialize(const string& config) override;
void Deinitialize() override;
Core::ProxyType<IContainer> Container(const string& id, IStringIterator& searchpaths,
const string& logpath, const string& configuration) override;

protected:
private:
void DestroyContainer(const string& name); // make sure that no leftovers from previous launch will cause crash
bool ContainerNameTaken(const string& name);
void containerStopCallback(int32_t cd, const std::string& containerId,
Expand Down