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

ShiftLeft pipeline support request by Fuming Zhang: Sync Tsp to GitHub task is removing files unexpectedly #5788

Closed
FumingZhang opened this issue Mar 23, 2023 · 5 comments
Assignees
Labels
Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo. Support

Comments

@FumingZhang
Copy link
Member

FumingZhang commented Mar 23, 2023

Task Sync Swagger to GitHub is removing some files unexpectedly.

The ADO PR #7796791 only changed AKS related content, but in the synced GitHub PR #11815 in addition to synchronizing necessary files, some files related to fleet are unexpectedly deleted.

From pipeline log, the fleet related files are recognized and included, but somehow deleted later.

I could help confirm the unexpectedly deleted files are there in both ADO source branch and ARMCoreRPDev.

We've defined manifest file like

"master": {
    "includedPaths": [
        "containerservice/*"
    ],
    "excludedPaths": [
        ".*/fleets.management/README.md",
        ".*/fleets.management/package.json",
        ".*/fleets.management/package-lock.json",
        ".*/fleets.management/.prettierrc.json"
    ]
}

More background

There are 2 sub-services under the Microsoft.ContainerService namespace, the first one is aks (with path /swagger/containerservice/resource-manager/Microsoft.ContainerService/aks) developed in the traditional way (modify the json file directly), and the second one is fleet (with path /swagger/containerservice/resource-manager/Microsoft.ContainerService/fleet) developed with cadl.

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Mar 23, 2023
@konrad-jamrozik
Copy link
Contributor

@FumingZhang big thx for the detailed report. I'll investigate during my business hours with priority. In the meantime, as before, as a workaround please submit PRs directly to the azure-rest-api-specs repo.

@konrad-jamrozik konrad-jamrozik self-assigned this Mar 23, 2023
@konrad-jamrozik konrad-jamrozik added Central-EngSys This issue is owned by the Engineering System team. Support and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Mar 23, 2023
@konrad-jamrozik
Copy link
Contributor

konrad-jamrozik commented Mar 27, 2023

Some of my findings so far:

The way it should work is:

  1. the directory swagger/containerservice is copied from aks-rp repo, branch haitao/list_k8s_swagger, to the directory specification/containerservice in azure-rest-api-specs-pr repo, branch containerservice/haitao/list_k8s_swagger
  2. A PR is made from the azure-rest-api-specs-pr repo, branch containerservice/haitao/list_k8s_swagger, into azure-rest-api-specs-pr repo, branch azure-rest-api-specs-pr:ARMCoreRPDev.

If you observe the links above, you will see that:

  • aks-rp:haitao/list_k8s_swagger has path /swagger/containerservice/fleets.management
  • azure-rest-api-specs-pr:containerservice/haitao/list_k8s_swagger does not have path /specification/containerservice/fleets.management`
  • azure-rest-api-specs-pr:ARMCoreRPDev has path /specification/containerservice/fleets.management

So the problem is in step 1., where the /fleets.management dir hasn't been properly copied. Only /resource-manager got copied.

The ShiftLeft pipeline definition states that cadlDirForArm: swagger/containerservice/fleets.management hence the issue is likely within SyncTspToGithub.ps1 task. Indeed, the log says:

Executing Remove-DeletedTsp src='/home/vsts/work/1/s/../Azure-azure-rest-api-specs-pr/swagger/containerservice/fleets.management' tar='specification/containerservice/fleets.management'
removing whole folder specification/containerservice/fleets.management

@konrad-jamrozik konrad-jamrozik changed the title ShiftLeft pipeline support request by Fuming Zhang: Sync Swagger to GitHub task is removing files unexpectedly ShiftLeft pipeline support request by Fuming Zhang: Sync Tsp to GitHub task is removing files unexpectedly Mar 27, 2023
@FumingZhang
Copy link
Member Author

BTW, I've got some new findings. I created ADO PR #7854193 with only one line changed in one commit, but there are two commits in the synced GitHub PR #11866, one by AutoSyncSwagger, exactly what was changed in the ADO PR, and the other suspicious one by AutoSyncTsp, which removes the fleet related files. 

Judging from the pipeline log, the tsp step does have a git commit operation (after rm dir specification/containerservice/fleets.management), maybe there are some problems here?

@konrad-jamrozik
Copy link
Contributor

@FumingZhang the bug should now be fixed, as I just merged this PR:

https://devdiv.visualstudio.com/DevDiv/_git/openapi-pipeline/pullrequest/461290

Unfortunately, I believe the PR affected by this bug will have to have a change pushed to it, as currently I am unaware of a way of triggering the pipeline again with Build.Reason set to PullRequest, which is required for proper execution of it.

@FumingZhang or @haitch could you try pushing a change to the PR https://msazure.visualstudio.com/CloudNativeCompute/_git/aks-rp/pullrequest/7796791
to see if it kicks off the ShiftLeft pipeline, with correct results this time?

This might not work due to the script logic being slightly changed and thus getting confused. In such case, I would kindly ask you to re-create the PR and see how the ShiftLeft pipeline behaves. I know it is a hassle, sorry!

@konrad-jamrozik
Copy link
Contributor

konrad-jamrozik commented Mar 28, 2023

With @FumingZhang we confirmed the fix works. Build log here. As a side note, it was possible to just re-trigger the PR pipeline with correct Build.Reason.

Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Central-EngSys This issue is owned by the Engineering System team. Spec PR Tools Tooling that runs in azure-rest-api-specs repo. Support
Projects
None yet
Development

No branches or pull requests

2 participants