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

Stages out of order in sdk.Deployment.Stages response from client.Create(...) #336

Closed
bhavenst opened this issue Jun 12, 2023 · 7 comments · Fixed by #341
Closed

Stages out of order in sdk.Deployment.Stages response from client.Create(...) #336

bhavenst opened this issue Jun 12, 2023 · 7 comments · Fixed by #341
Assignees
Labels
bug Something isn't working v1

Comments

@bhavenst
Copy link

The stage array is coming back with the stages in almost reverse order though not quite. This is the order:

AAP Applications
Seeded Content
Billing
Database Server and Databases
Private DNS
AAP Operators
AKS Cluster
VNET and Subnets
Application Ingress
AAP Repository

It should be:
VNET
Private DNS
Database
AAP Repo
AKS Cluster
AAP Operators
AAP Applications
App Ingress
Seeded
Billing

A couple of those could be reversed since they can run in parallel, but that's the basic proper order.

To display these properly (i.e. execution starting from the top and working down) they need to be in order coming back from the Create function. Either that or some dependency ordering metadata included so that we can sort them on our end.

@kevinhillinger
Copy link
Member

kevinhillinger commented Jun 13, 2023

We cannot guaranteed event message ordering (if this was inferred by "ordering"), and the nested templates will get scheduled on Azure according to the template's dependsOn structure.

What we can do is deliver the message with a timestamp of startedAt

startedAt = the time we identified that the stage was started by Azure.

when Azure signals that the deployment for a Stage was accepted, we will capture that and signal with an event message of deploymentStarted or stageStarted

@kevinhillinger kevinhillinger linked a pull request Jun 13, 2023 that will close this issue
@kevinhillinger kevinhillinger removed a link to a pull request Jun 13, 2023
@kevinhillinger
Copy link
Member

It will be up to the driver to handle visual ordering of the Stages.

@kevinhillinger
Copy link
Member

What can be done is setting attributes on the nestedTemplate tags with the prefix publisher.

These will be parsed and set on the stage record. Example:

{
  "id": 1,
  "name": "TaggedDeployment",
  "stages": [
    {
      "attributes": {},
      "deploymentName": "[variables('deploymentName5')]",
      "id": "4809d787-3dd4-45c9-a84a-215d0030c77b",
      "name": "storageAccounts",
      "retries": 3
    },
    {
      "attributes": {
        "publisher.ordinal": "1"
      },
      "deploymentName": "[variables('deploymentName5')]",
      "id": "22e9f9a0-9fd2-4294-a0a3-0101246d9700",
      "name": "storageAccounts2",
      "retries": 3
    },
    {
      "attributes": {},
      "deploymentName": "[variables('deploymentName5')]",
      "id": "33e9f9a0-9fd2-4294-a0a3-0101246d9700",
      "name": "storageAccounts3",
      "retries": 3
    }
  ],

In this example, the publisher.ordinal is defined in the template submitted. MODM will not do anything with these values, and are only for publisher/vendor use.

This way, you don't have to store additional information if you don't want to and use these values to for display ordering purposes.

@kevinhillinger
Copy link
Member

@kevinhillinger kevinhillinger linked a pull request Jun 13, 2023 that will close this issue
bobjac added a commit that referenced this issue Jun 13, 2023
* fixing dependsOn template

* adding status pending and forcing OperationTask implementations

* adding publisher attributes

* mapping attributes to model.Stage

* regen for attributes

* updating template

* generating missing properties

* mapping deployment properties to SDK model

---------

Co-authored-by: Bob Jacobs <bob.jacobs@outlook.com>
@tznamena
Copy link
Contributor

I don't think this issue was fixed, it was simply thrown back on caller to handle or suggests to add more complexity. What is the point of MODM then?

@kevinhillinger
Copy link
Member

kevinhillinger commented Jun 13, 2023

@tznamena

Input is very important to us. Your comment is direct, but the SDK private preview was made visible in March and has been published since April and never reflected ordering. MODM Stages are identified by the use of modm.id tag set on a nested template in an ARM Template. a UUID value set on this tag value is the specification for this feature (UUIDs are inherently in no order or sequence).

If what you're trying to ask for is how to represent Stages in a particular visual order in your UI, one way it can be done is by a publisher attribute a value set through the publisher tag on the nested template. Use the prefix publisher. to identify it. MODM will take these values in and read it back in SDK calls. They are just there for metadata use by the caller, and as shown will be delivered on the corresponding Stage.

No data would need to be persisted to a DB, just call the SDK to get the stages and sort. This can be done inside an HTTP handler with a helper function. Here's a gist:

// new up SDK client, then call:

response, _ := client.Get(c.Request().Context(), deploymentId)
stages := response.Deployment.Stages

getOrder := func(s *sdk.DeploymentStage) int {
	value := getAttrValue(s, "publisher.order") // ex. helper that gets string value
	if value == "" {
		return 0
	}
	i, err := strconv.Atoi(value)
	if err != nil {
		return 0
	}
	return i
}

sort.Slice(stages, func(i, j int) bool {
	a := getOrder(stages[i])
	b := getOrder(stages[j])
	return a < b
})

return c.JSON(http.StatusOK, stages)

return stages

Since this doesn't change, it could be stored as a package level variable and cached for fast retrieval.

@tznamena
Copy link
Contributor

Is it completely unreasonable request for MODM to return stages with order number directly in stage?

Our implementation before MODM had code to figure out order of templates. Now with MODM we don't need to split the templates up BUT have to figure out the order of stages when they come back. So the promise of MODM simplifying this part of our implementation is not fulfilled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants