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

Make control interface optional for a workload #353

Merged
merged 27 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
51a0287
Fix failing stests
HorjuRares Aug 12, 2024
e345e63
First attempt at defining the necessary stests
HorjuRares Aug 13, 2024
e83e4b9
Revert "Fix failing stests"
HorjuRares Aug 21, 2024
fe02eb0
Merge branch 'eclipse-ankaios:main' into 322_control_interface
HorjuRares Aug 21, 2024
b665936
Merge branch 'eclipse-ankaios:main' into 322_control_interface
GabyUnalaq Aug 28, 2024
51cf118
Initial implementation of optional control interface
GabyUnalaq Aug 28, 2024
fd0eba8
Fix control interface path and creation logic
GabyUnalaq Aug 28, 2024
39afd9f
Add tests for start-up and update without control interface
HorjuRares Aug 26, 2024
718a44f
Modify control interface optional stests
HorjuRares Aug 28, 2024
3a475a3
Merge branch 'eclipse-ankaios:main' into 322_control_interface
HorjuRares Aug 29, 2024
def3c19
Update: all three control interface system tests are running
HorjuRares Aug 29, 2024
6a23bcf
Update: Refactor the check of mount point creation
HorjuRares Aug 29, 2024
3ad894b
Update: checking for all workloads of an agent
HorjuRares Aug 30, 2024
4d00217
Small update on workload indexing in mount point checking
HorjuRares Aug 30, 2024
0f6d8c2
Remove unused variables
HorjuRares Aug 30, 2024
0bffaf3
Fix unit tests and other small changes
GabyUnalaq Aug 30, 2024
760a285
Add more unit tests for control interface creation logic
GabyUnalaq Sep 2, 2024
3dc1477
Add and link requirements
GabyUnalaq Sep 3, 2024
2b5dfe1
Merge branch 'main' into 322_control_interface
GabyUnalaq Sep 3, 2024
6fc9525
Fix image registry from config
GabyUnalaq Sep 4, 2024
ba04f64
Fix attempt: authorization - no rules system test
HorjuRares Sep 4, 2024
918b198
Update control interface tester tag
HorjuRares Sep 4, 2024
e02add9
Update control interface stests for mount point existence verification
HorjuRares Sep 4, 2024
36cb457
Fix findings
GabyUnalaq Sep 6, 2024
cc2cbee
Merge branch 'eclipse-ankaios:main' into 322_control_interface
HorjuRares Sep 9, 2024
2d72621
Check for no access to Control Interface when no rules are being defined
HorjuRares Sep 9, 2024
8b3f970
Fix requirement findings
GabyUnalaq Sep 10, 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
27 changes: 24 additions & 3 deletions agent/doc/swdesign/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,27 @@ Needs:
- impl
- utest

#### Control Interface created for eligible workloads
`swdd~agent-control-interface-created-for-eligible-workloads~1`

Status: approved

When the workload has control interface access rules configured, the Control Interface shall be created for that workload.

Comment:
Due to the logic that by default, the pipes are restricted, it makes sense to check only the allowed rules.

Rationale:
Creating a control interface affects the start-up time of a workload and thus it should be created only if it's used.

Tags:
- ControlInterface

Needs:
- impl
- utest
- stest

#### Agent skips unknown runtime
`swdd~agent-skips-unknown-runtime~1`

Expand Down Expand Up @@ -658,7 +679,7 @@ Status: approved

When the RuntimeFacade gets a requests to create a workload, the RuntimeFacade shall:
* start the WorkloadControlLoop waiting for WorkloadCommands
* create a new ControlInterface instance for the new workload
* create a new ControlInterface instance for the new workload if the workload has access rules configured
* request the create of the workload by sending a create command to the WorkloadControlLoop
* return a new workload object containing a WorkloadCommandSender to communicate with the WorkloadControlLoop

Expand Down Expand Up @@ -866,8 +887,8 @@ Status: approved
When the WorkloadObject receives a trigger to update the workload, it:
* triggers a comparison of the existing and new control interface metadata
* stops the old control interface if the comparison returns that the metadata has changed
* creates a new ControlInterface instance if the comparison returns that the metadata has changed
* stores the new ControlInterface instance after the creation
* creates a new ControlInterface instance if the comparison returns that the metadata has changed and if access rules are configured
* stores the new ControlInterface instance instead of the old one
* sends a command via the WorkloadCommandSender to the WorkloadControlLoop to update the workload

Tags:
Expand Down
6 changes: 3 additions & 3 deletions agent/src/control_interface/control_interface_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ use crate::control_interface::ControlInterface;

pub struct ControlInterfaceInfo {
run_folder: PathBuf,
pub workload_instance_name: WorkloadInstanceName,
workload_instance_name: WorkloadInstanceName,
#[cfg_attr(test, allow(dead_code))]
pub control_interface_to_server_sender: ToServerSender,
pub authorizer: Authorizer,
control_interface_to_server_sender: ToServerSender,
authorizer: Authorizer,
}

#[cfg_attr(test, automock)]
Expand Down
162 changes: 137 additions & 25 deletions agent/src/runtime_connectors/runtime_facade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub trait RuntimeFacade: Send + Sync + 'static {
fn resume_workload(
&self,
runtime_workload: WorkloadSpec,
control_interface: Option<ControlInterface>,
control_interface: Option<ControlInterfaceInfo>,
update_state_tx: &WorkloadStateSender,
) -> Workload;

Expand Down Expand Up @@ -112,13 +112,13 @@ impl<
fn resume_workload(
&self,
workload_spec: WorkloadSpec,
control_interface: Option<ControlInterface>,
control_interface_info: Option<ControlInterfaceInfo>,
update_state_tx: &WorkloadStateSender,
) -> Workload {
let (_task_handle, workload) = Self::resume_workload_non_blocking(
self,
workload_spec,
control_interface,
control_interface_info,
update_state_tx,
);
workload
Expand Down Expand Up @@ -155,39 +155,43 @@ impl<
) -> (JoinHandle<()>, Workload) {
let runtime = self.runtime.to_owned();
let update_state_tx = update_state_tx.clone();
let workload_name = workload_spec.instance_name.workload_name().to_owned();

let (control_interface_path, control_interface) = if let Some(info) = control_interface_info
{
let run_folder = info.get_run_folder().clone();
let output_pipe_sender = info.get_to_server_sender();
let instance_name = info.get_instance_name().clone();
let authorizer = info.move_authorizer();
let control_interface = match ControlInterface::new(
&run_folder,
&instance_name,
output_pipe_sender,
authorizer,
) {
Ok(control_interface) => Some(control_interface),
match ControlInterface::new(&run_folder, &instance_name, output_pipe_sender, authorizer)
krucod3 marked this conversation as resolved.
Show resolved Hide resolved
{
Ok(result) => {
log::info!(
"Successfully created control interface for workload '{}'.",
workload_name
);
(
Some(workload_spec.instance_name.pipes_folder_name(&run_folder)),
Some(result),
)
}
Err(err) => {
log::warn!(
"Could not create control interface when creating workload '{}': '{}'",
instance_name,
workload_name,
err
);
None
(None, None)
}
};

(
Some(workload_spec.instance_name.pipes_folder_name(&run_folder)),
control_interface,
)
}
} else {
log::debug!(
"Skipping creation of control interface for workload '{}'.",
workload_name
);
(None, None)
};

let workload_name = workload_spec.instance_name.workload_name().to_owned();
log::debug!(
"Creating '{}' workload '{}'.",
runtime.name(),
Expand Down Expand Up @@ -226,7 +230,7 @@ impl<
fn resume_workload_non_blocking(
&self,
workload_spec: WorkloadSpec,
control_interface: Option<ControlInterface>,
control_interface_info: Option<ControlInterfaceInfo>,
update_state_tx: &WorkloadStateSender,
) -> (JoinHandle<()>, Workload) {
let workload_name = workload_spec.instance_name.workload_name().to_owned();
Expand All @@ -239,6 +243,32 @@ impl<
workload_name,
);

// [impl->swdd~agent-control-interface-created-for-eligible-workloads~1]
let control_interface = control_interface_info.and_then(|info| { if workload_spec.needs_control_interface() {
let run_folder = info.get_run_folder().clone();
let output_pipe_sender = info.get_to_server_sender();
let instance_name = info.get_instance_name().clone();
let authorizer = info.move_authorizer();
match ControlInterface::new(&run_folder, &instance_name, output_pipe_sender, authorizer)
{
Ok(result) => Some(result),
Err(err) => {
log::warn!(
"Could not reuse or create control interface when resuming workload '{}': '{}'",
workload_spec.instance_name,
err
);
None
}
}
} else {
log::info!(
"No control interface access rights specified for workload '{}'. Skipping creation of control interface.",
workload_spec.instance_name.clone().workload_name()
);
None
}});

let (workload_command_tx, workload_command_receiver) = WorkloadCommandSender::new();
let workload_command_sender = workload_command_tx.clone();
let task_handle = tokio::spawn(async move {
Expand Down Expand Up @@ -335,7 +365,9 @@ impl<
#[cfg(test)]
mod tests {
use common::objects::{
generate_test_workload_spec_with_param, ExecutionState, WorkloadInstanceName, WorkloadState,
generate_test_workload_spec_with_control_interface_access,
generate_test_workload_spec_with_param, ExecutionState, WorkloadInstanceName,
WorkloadState,
};

use crate::{
Expand Down Expand Up @@ -408,7 +440,7 @@ mod tests {
.get_lock_async()
.await;

let workload_spec = generate_test_workload_spec_with_param(
let workload_spec = generate_test_workload_spec_with_control_interface_access(
AGENT_NAME.to_string(),
WORKLOAD_1_NAME.to_string(),
RUNTIME_NAME.to_string(),
Expand Down Expand Up @@ -480,13 +512,93 @@ mod tests {
}

// [utest->swdd~agent-resume-workload~2]
// [utest->swdd~agent-control-interface-created-for-eligible-workloads~1]
#[tokio::test]
async fn utest_runtime_facade_resume_workload() {
async fn utest_runtime_facade_resume_workload_with_control_interface_access() {
let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC
.get_lock_async()
.await;

let control_interface_mock = MockControlInterface::default();
let mut control_interface_info_mock = MockControlInterfaceInfo::default();
control_interface_info_mock
.expect_get_run_folder()
.once()
.return_const(PIPES_LOCATION.into());
control_interface_info_mock
.expect_get_to_server_sender()
.once()
.return_const(tokio::sync::mpsc::channel::<common::to_server_interface::ToServer>(1).0);
control_interface_info_mock
.expect_get_instance_name()
.once()
.return_const(
WorkloadInstanceName::builder()
.workload_name(WORKLOAD_1_NAME)
.build(),
);
control_interface_info_mock
.expect_move_authorizer()
.once()
.return_once(MockAuthorizer::default);

let control_interface_new_context = MockControlInterface::new_context();
control_interface_new_context
.expect()
.once()
.return_once(|_, _, _, _| Ok(MockControlInterface::default()));

let workload_spec = generate_test_workload_spec_with_control_interface_access(
AGENT_NAME.to_string(),
WORKLOAD_1_NAME.to_string(),
RUNTIME_NAME.to_string(),
);

let (wl_state_sender, _wl_state_receiver) =
tokio::sync::mpsc::channel(TEST_CHANNEL_BUFFER_SIZE);

let mock_control_loop = MockWorkloadControlLoop::run_context();
mock_control_loop
.expect()
.once()
.return_once(|_: ControlLoopState<String, StubStateChecker>| ());

let mock_workload = MockWorkload::default();
let new_workload_context = MockWorkload::new_context();
new_workload_context
.expect()
.once()
.return_once(|_, _, _| mock_workload);

let runtime_mock = MockRuntimeConnector::new();

let ownable_runtime_mock: Box<dyn OwnableRuntime<String, StubStateChecker>> =
Box::new(runtime_mock.clone());
let test_runtime_facade = Box::new(GenericRuntimeFacade::<String, StubStateChecker>::new(
ownable_runtime_mock,
));

let (task_handle, _workload) = test_runtime_facade.resume_workload_non_blocking(
workload_spec.clone(),
Some(control_interface_info_mock),
&wl_state_sender,
);

tokio::task::yield_now().await;
assert!(task_handle.await.is_ok());
runtime_mock.assert_all_expectations().await;
}

// [utest->swdd~agent-control-interface-created-for-eligible-workloads~1]
#[tokio::test]
async fn utest_runtime_facade_resume_workload_without_control_interface_access() {
let _guard = crate::test_helper::MOCKALL_CONTEXT_SYNC
.get_lock_async()
.await;

let control_interface_info_mock = MockControlInterfaceInfo::default();

let control_interface_new_context = MockControlInterface::new_context();
control_interface_new_context.expect().never();

let workload_spec = generate_test_workload_spec_with_param(
AGENT_NAME.to_string(),
Expand Down Expand Up @@ -520,7 +632,7 @@ mod tests {

let (task_handle, _workload) = test_runtime_facade.resume_workload_non_blocking(
workload_spec.clone(),
Some(control_interface_mock),
Some(control_interface_info_mock),
&wl_state_sender,
);

Expand Down
Loading