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

Add a unique step id for each OperationStep #3239

Merged
merged 12 commits into from
Feb 28, 2023
2 changes: 1 addition & 1 deletion api_app/_version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "0.13.1"
__version__ = "0.13.2"
10 changes: 6 additions & 4 deletions api_app/db/repositories/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ def create_operation_id() -> str:

def create_main_step(self, resource_template: dict, action: str, resource_id: str, status: Status, message: str) -> OperationStep:
return OperationStep(
stepId="main",
id=str(uuid.uuid4()),
templateStepId="main",
stepTitle=f"Main step for {resource_id}",
resourceId=resource_id,
resourceTemplateName=resource_template["name"],
resourceType=resource_template["resourceType"],
resourceAction=action,
parentResourceId=resource_id,
sourceTemplateResourceId=resource_id,
status=status,
message=message,
updatedWhen=self.get_timestamp())
Expand Down Expand Up @@ -133,7 +134,8 @@ async def build_step_list(self, steps: List[OperationStep], resource_template_di
resource_for_step_status, resource_for_step_message = self.get_initial_status(step["resourceAction"])

steps.append(OperationStep(
stepId=step["stepId"],
id=str(uuid.uuid4()),
templateStepId=step["stepId"],
stepTitle=step["stepTitle"],
resourceId=resource_for_step.id,
resourceTemplateName=resource_for_step.templateName,
Expand All @@ -142,7 +144,7 @@ async def build_step_list(self, steps: List[OperationStep], resource_template_di
status=resource_for_step_status,
message=resource_for_step_message,
updatedWhen=self.get_timestamp(),
parentResourceId=resource_id
sourceTemplateResourceId=resource_id
))
return steps

Expand Down
6 changes: 4 additions & 2 deletions api_app/models/domain/operation.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ class OperationStep(AzureTREModel):
The steps are built up front as the operation is created from the initial user request.
As each step completes, the next one is processed.
"""
stepId: str = Field(title="stepId", description="Unique id identifying the step")
id: str = Field(title="Id", description="Unique id identifying the step")
templateStepId: str = Field(title="templateStepId", description="Unique id identifying the step")
stepTitle: Optional[str] = Field(title="stepTitle", description="Human readable title of what the step is for")
resourceId: Optional[str] = Field(title="resourceId", description="Id of the resource to update")
resourceTemplateName: Optional[str] = Field("", title="resourceTemplateName", description="Name of the template for the resource under change")
Expand All @@ -51,7 +52,8 @@ class OperationStep(AzureTREModel):
status: Optional[Status] = Field(None, title="Operation step status")
message: Optional[str] = Field("", title="Additional operation step status information")
updatedWhen: Optional[float] = Field("", title="POSIX Timestamp for When the operation step was updated")
parentResourceId: Optional[str] = Field(title="parentResourceId", description="Id of the parent of the resource to update")
# An example for this property will be if we have a step that is responsible for updating the firewall, and its origin was the guacamole workspace service, the id here will be the guacamole id
sourceTemplateResourceId: Optional[str] = Field(title="sourceTemplateResourceId", description="Id of the parent of the resource to update")

def is_success(self) -> bool:
return self.status in (
Expand Down
13 changes: 7 additions & 6 deletions api_app/service_bus/deployment_status_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,8 @@ async def update_status_in_database(self, message: DeploymentStatusUpdateMessage

current_step_index = 0
for i, step in enumerate(operation.steps):
if step.stepId == message.stepId and step.resourceId == str(message.id):
# TODO more simple condition
if step.id == message.stepId and step.resourceId == str(message.id):
step_to_update = step
current_step_index = i
if i == (len(operation.steps) - 1):
Expand Down Expand Up @@ -146,7 +147,7 @@ async def update_status_in_database(self, message: DeploymentStatusUpdateMessage
# catch any errors in updating the resource - maybe Cosmos / schema invalid etc, and report them back to the op
try:
# parent resource is always retrieved via cosmos, hence it is always with redacted sensitive values
parent_resource = await self.resource_repo.get_resource_by_id(next_step.parentResourceId)
parent_resource = await self.resource_repo.get_resource_by_id(next_step.sourceTemplateResourceId)
resource_to_send = await update_resource_for_step(
operation_step=next_step,
resource_repo=self.resource_repo,
Expand All @@ -159,8 +160,8 @@ async def update_status_in_database(self, message: DeploymentStatusUpdateMessage
user=operation.user)

# create + send the message
logging.info(f"Sending next step in operation to deployment queue -> step_id: {next_step.stepId}, action: {next_step.resourceAction}")
content = json.dumps(resource_to_send.get_resource_request_message_payload(operation_id=operation.id, step_id=next_step.stepId, action=next_step.resourceAction))
logging.info(f"Sending next step in operation to deployment queue -> step_id: {next_step.templateStepId}, action: {next_step.resourceAction}")
content = json.dumps(resource_to_send.get_resource_request_message_payload(operation_id=operation.id, step_id=next_step.id, action=next_step.resourceAction))
await send_deployment_message(content=content, correlation_id=operation.id, session_id=resource_to_send.id, action=next_step.resourceAction)
except Exception as e:
logging.exception("Unable to send update for resource in pipeline step")
Expand Down Expand Up @@ -194,12 +195,12 @@ async def update_overall_operation_status(self, operation: Operation, step: Oper

if step.is_failure():
operation.status = self.get_failure_status_for_action(operation.action)
operation.message = f"Multi step pipeline failed on step {step.stepId}"
operation.message = f"Multi step pipeline failed on step {step.templateStepId}"

# pipeline failed - update the primary resource (from the main step) as failed too
main_step = None
for i, step in enumerate(operation.steps):
if step.stepId == "main":
if step.templateStepId == "main":
main_step = step
break

Expand Down
6 changes: 3 additions & 3 deletions api_app/service_bus/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async def update_resource_for_step(operation_step: OperationStep, resource_repo:
# step_resource is the resource instance where the step was defined. e.g. 'add firewall rule' step defined in Guacamole template -> the step_resource is the Guacamole ws service.
# root_resource is theresource on which the user chose to update, i.e. the top most resource in cascaded action or the same resource in a non-cascaded action.
if step_resource is None:
step_resource = await resource_repo.get_resource_by_id(operation_step.parentResourceId)
step_resource = await resource_repo.get_resource_by_id(operation_step.sourceTemplateResourceId)

# If we are handling the root resource, we can leverage the given resource which has non redacted properties
if root_resource is not None and root_resource.id == step_resource.id:
Expand Down Expand Up @@ -73,12 +73,12 @@ async def update_resource_for_step(operation_step: OperationStep, resource_repo:
# get the template step
template_step = None
for step in parent_template.pipeline.dict()[primary_action]:
if step["stepId"] == operation_step.stepId:
if step["stepId"] == operation_step.templateStepId:
template_step = parse_obj_as(PipelineStep, step)
break

if template_step is None:
raise Exception(f"Cannot find step with id of {operation_step.stepId} in template {step_resource.templateName} for action {primary_action}")
raise Exception(f"Cannot find step with id of {operation_step.templateStepId} in template {step_resource.templateName} for action {primary_action}")

resource_to_send = await try_update_with_retries(
num_retries=3,
Expand Down
2 changes: 1 addition & 1 deletion api_app/service_bus/resource_request_sender.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ async def send_resource_request_message(resource: Resource, operations_repo: Ope
user=user)

# create + send the message
content = json.dumps(resource_to_send.get_resource_request_message_payload(operation_id=operation.id, step_id=first_step.stepId, action=first_step.resourceAction))
content = json.dumps(resource_to_send.get_resource_request_message_payload(operation_id=operation.id, step_id=first_step.id, action=first_step.resourceAction))
await send_deployment_message(content=content, correlation_id=operation.id, session_id=first_step.resourceId, action=first_step.resourceAction)

return operation
15 changes: 9 additions & 6 deletions api_app/tests_ma/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,8 @@ def multi_step_operation(
updatedWhen=FAKE_CREATE_TIMESTAMP,
steps=[
OperationStep(
stepId="pre-step-1",
id="random-uuid-1",
templateStepId="pre-step-1",
stepTitle="Title for pre-step-1",
resourceAction="upgrade",
resourceTemplateName=basic_shared_service_template.name,
Expand All @@ -376,10 +377,11 @@ def multi_step_operation(
status=Status.AwaitingUpdate,
message="This resource is waiting to be updated",
updatedWhen=FAKE_CREATE_TIMESTAMP,
parentResourceId="59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76"
sourceTemplateResourceId="59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76"
),
OperationStep(
stepId="main",
id="random-uuid-2",
templateStepId="main",
stepTitle="Main step for 59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76",
resourceAction="install",
resourceType=ResourceType.Workspace,
Expand All @@ -388,10 +390,11 @@ def multi_step_operation(
status=Status.AwaitingDeployment,
message="This resource is waiting to be deployed",
updatedWhen=FAKE_CREATE_TIMESTAMP,
parentResourceId="59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76"
sourceTemplateResourceId="59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76"
),
OperationStep(
stepId="post-step-1",
id="random-uuid-3",
templateStepId="post-step-1",
stepTitle="Title for post-step-1",
resourceAction="upgrade",
resourceType=basic_shared_service_template.resourceType,
Expand All @@ -400,7 +403,7 @@ def multi_step_operation(
status=Status.AwaitingUpdate,
message="This resource is waiting to be updated",
updatedWhen=FAKE_CREATE_TIMESTAMP,
parentResourceId="59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76"
sourceTemplateResourceId="59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76"
),
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,15 @@ def sample_resource_operation(resource_id: str, operation_id: str):
user=create_test_user(),
steps=[
OperationStep(
stepId="main",
id="random-uuid-1",
templateStepId="main",
stepTitle="Main step for resource-id",
resourceAction="install",
resourceType=ResourceType.Workspace,
resourceTemplateName="template1",
resourceId=resource_id,
updatedWhen=FAKE_CREATE_TIMESTAMP
updatedWhen=FAKE_CREATE_TIMESTAMP,
sourceTemplateResourceId=resource_id
)
]
)
Expand Down Expand Up @@ -270,7 +272,7 @@ async def test_send_uninstall_message_raises_503_on_service_bus_exception(self,
@pytest.mark.asyncio
async def test_save_and_deploy_masks_secrets(self, send_deployment_message_mock, resource_template_repo, resource_repo, operations_repo, basic_resource_template, resource_history_repo):
resource = sample_resource_with_secret()
step_id = "main"
step_id = "random-uuid-1"
operation_id = str(uuid.uuid4())
operation = sample_resource_operation(resource_id=resource.id, operation_id=operation_id)

Expand Down
6 changes: 4 additions & 2 deletions api_app/tests_ma/test_api/test_routes/test_workspaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,12 @@ def sample_resource_operation(resource_id: str, operation_id: str):
user=create_test_user(),
steps=[
OperationStep(
stepId="main",
id="random-uuid",
templateStepId="main",
resourceId=resource_id,
resourceAction="install",
updatedWhen=FAKE_UPDATE_TIMESTAMP
updatedWhen=FAKE_UPDATE_TIMESTAMP,
sourceTemplateResourceId=resource_id
)
]
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ async def resource_template_repo():
yield resource_template_repo


@patch('uuid.uuid4', side_effect=["random-uuid-1", "random-uuid-2", "random-uuid-3"])
@patch("db.repositories.operations.OperationRepository.get_timestamp", return_value=FAKE_CREATE_TIMESTAMP)
@patch("db.repositories.operations.OperationRepository.create_operation_id", return_value=OPERATION_ID)
async def test_create_operation_steps_from_multi_step_template(_, __, resource_repo, test_user, multi_step_operation, operations_repo, basic_shared_service, resource_template_repo, multi_step_resource_template):
async def test_create_operation_steps_from_multi_step_template(_, __, ___, resource_repo, test_user, multi_step_operation, operations_repo, basic_shared_service, resource_template_repo, multi_step_resource_template):

expected_op = multi_step_operation
expected_op.id = OPERATION_ID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

test_sb_message = {
"operationId": OPERATION_ID,
"stepId": "main",
"stepId": "random-uuid",
"id": "59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76",
"status": Status.Deployed,
"message": "test message",
Expand All @@ -36,7 +36,7 @@

test_sb_message_with_outputs = {
"operationId": OPERATION_ID,
"stepId": "main",
"stepId": "random-uuid",
"id": "59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76",
"status": Status.Deployed,
"message": "test message",
Expand All @@ -48,15 +48,15 @@

test_sb_message_multi_step_1_complete = {
"operationId": OPERATION_ID,
"stepId": "pre-step-1",
"stepId": "random-uuid-1",
"id": "59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76",
"status": Status.Updated,
"message": "upgrade succeeded"
}

test_sb_message_multi_step_3_complete = {
"operationId": OPERATION_ID,
"stepId": "post-step-1",
"stepId": "random-uuid-3",
"id": "59b5c8e7-5c42-4fcb-a7fd-294cfc27aa76",
"status": Status.Updated,
"message": "upgrade succeeded"
Expand Down Expand Up @@ -96,13 +96,15 @@ def create_sample_operation(resource_id, request_action):
updatedWhen=FAKE_UPDATE_TIMESTAMP,
steps=[
OperationStep(
stepId="main",
id="random-uuid",
templateStepId="main",
resourceId=resource_id,
stepTitle=f"main step for {resource_id}",
resourceTemplateName="workspace-base",
resourceType=ResourceType.Workspace,
resourceAction=request_action,
updatedWhen=FAKE_UPDATE_TIMESTAMP
updatedWhen=FAKE_UPDATE_TIMESTAMP,
sourceTemplateResourceId=resource_id
)
]
)
Expand Down
2 changes: 1 addition & 1 deletion ui/app/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "tre-ui",
"version": "0.4.1",
"version": "0.4.2",
"private": true,
"dependencies": {
"@azure/msal-browser": "^2.32.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export const NotificationItem: React.FunctionComponent<NotificationItemProps> =
<Stack horizontal style={{ marginTop: '10px' }}>
<Stack.Item grow={5}>
{
props.operation.steps && props.operation.steps.length > 0 && !(props.operation.steps.length === 1 && props.operation.steps[0].stepId === 'main') ?
props.operation.steps && props.operation.steps.length > 0 && !(props.operation.steps.length === 1 && props.operation.steps[0].templateStepId === 'main') ?
<FluentLink title={isExpanded ? 'Show less' : 'Show more'} href="#" onClick={() => { setIsExpanded(!isExpanded) }} style={{ position: 'relative', top: '2px' }}>{isExpanded ? <Icon iconName='ChevronUp' aria-label='Expand Steps' /> : <Icon iconName='ChevronDown' aria-label='Collapse Steps' />}</FluentLink>
:
' '
Expand All @@ -150,7 +150,7 @@ export const NotificationItem: React.FunctionComponent<NotificationItemProps> =
<li key={i}>
<Icon iconName={getIconAndColourForStatus(s.status)[0]} style={{ color: getIconAndColourForStatus(s.status)[1], position: 'relative', top: '2px', marginRight: '10px' }} />
{
s.stepId === "main" ?
s.templateStepId === "main" ?
<>{notification.resource.properties.display_name}: {props.operation.action}</> :
s.stepTitle
}
Expand Down
2 changes: 1 addition & 1 deletion ui/app/src/models/operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface Operation {
}

export interface OperationStep {
stepId: string,
templateStepId: string,
stepTitle: string,
resourceId: string,
resourceTemplateName: string,
Expand Down