Skip to content

Commit

Permalink
Add a unique step id for each OperationStep (#3239)
Browse files Browse the repository at this point in the history
* initial working impl

* test fixes

* bump api version

* bump ui version

* rename to templateStepId

* rename to sourceTemplateResourceId

* rename sourceTemplateResourceId

* add an example for  sourceTemplateResourceId

---------

Co-authored-by: Anat Balzam <anat@example.com>
Co-authored-by: Tamir Kamara <26870601+tamirkamara@users.noreply.github.com>
Co-authored-by: Anat Balzam <>
  • Loading branch information
3 people authored Feb 28, 2023
1 parent e813221 commit 61737f3
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 39 deletions.
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

0 comments on commit 61737f3

Please sign in to comment.