Add external deployment ID #3213
Conversation
…-linuxappdown-deeplink Update deep link to LinuxAppDown
…add-external-deployment-id
|
|
Merge into fork to remain up-to-date
| @@ -183,6 +183,20 @@ public void Delete(string id) | |||
| } | |||
| } | |||
|
|
|||
| if (!string.IsNullOrEmpty(deploymentInfo.FixedDeploymentId)) | |||
| { | |||
| changeSet = new ChangeSet( | |||
There was a problem hiding this comment.
Should we not create the original ChangeSet with the appropriate Deployment ID instead of a GUID?
There was a problem hiding this comment.
(let's continue this conversation on the kudulite thread)
| string deploymentId = null; | ||
| IEnumerable<string> idValues; | ||
|
|
||
| if (Request.Headers.TryGetValues(Constants.ScmDeploymentIdHeader, out idValues) && idValues.Count() > 0) | ||
| { | ||
| deploymentId = idValues.ElementAt(0); | ||
| } | ||
|
|
There was a problem hiding this comment.
Would be good to have a helper method for doing this like the StatusCode400() method in this class.
Comment resolution Fixed PostDeploymentHelper error
FE Kudu hook changes
| { | ||
| // Since build is success we need to force restart on windows | ||
| // Directly set this instead of Constants.BuildSuccessful | ||
| updateStatusObj.DeploymentStatus = Constants.PostBuildRestartRequired; |
There was a problem hiding this comment.
We should do this from RestartMainSiteIfNeeded() by substituting old logic. But because Windows flow is not fully implemented, hide it behind an app setting for now.
Requesting a restart here has two problems. 1. We will end up doing a restart even if restart is disabled using /api/publish?restart=false. 2. RestartMainSiteIfNeeded() will do a restart once and we will end up doing another restart here.
@jvano @JasonFreeberg - FYI
There was a problem hiding this comment.
Notes from 10/12 meeting concerning the above:
- Adjust the hook to check for the custom deployment ID for code apps. If the custom deployment ID exists, then we don't do the restart callback
- In that case we know that the client is using the new deployment flow, and the restart will be handled at controller
| if (string.IsNullOrEmpty(deploymentInfo.DeploymentId)) | ||
| { | ||
| await RestartMainSiteIfNeeded(tracer, logger, deploymentInfo); | ||
| } |
There was a problem hiding this comment.
Would be better to add this check for DeploymentId along with other checks inside RestartmainSiteIfNeeded. That way RestartmainSiteIfNeeded can be called unconditionally and as the name says - the site will be restarted only if needed.
Also, you may want to consider posting the restart operation to the FE from within RestartMainSiteIfNeeded. RestartmainSiteIfNeeded already has 2 ways of requesting a restart. One way is using a filesystem based notification. Second is using the /api/restart API. The new one being introduced is the third way and unless there is a good reason it would be good to do it from within RestartMainSiteIfNeeded. That way callers can call RestartMainSiteIfNeeded without having to worry about the details. Also - this keeps it consistent with KuduLite.
There was a problem hiding this comment.
Also, make sure that:
- /api/publish?restart=false does not trigger a restart (neither the old way nor new way of restarting)
- The new restart behavior is enabled only after the controller changes for Windows are live in PROD. Until then, all deployment flows should use the old restart behavior.
@jvano @JasonFreeberg FYI
suwatch
force-pushed
the
master
branch
2 times, most recently
from
39d07e9
to
186dc01
Compare
suwatch
force-pushed
the
master
branch
3 times, most recently
from
4b7eb01
to
05c0261
Compare
suwatch
force-pushed
the
master
branch
4 times, most recently
from
75ccbe8
to
9c865c3
Compare
…ploystatus Fail silently if call to update deployment status fails
Return bool to indicate success or failure on db operation through FE
Move first BuildRequestReceived from FE to Kudu
Full proof check if we need to fallback to next restart method throug…
|
Please squash to one commit and rebase with latest master - let me know when done. |
Create new GUID deployment ID for async requests and add to header
In zipdeploy/wardeploy, check incoming headers for a fixed deployment ID, and use it if present.