-
Notifications
You must be signed in to change notification settings - Fork 558
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
[azp]: Add Azp for DASH #2501
[azp]: Add Azp for DASH #2501
Conversation
Signed-off-by: Ze Gan <ganze718@gmail.com>
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
.azure-pipelines/build-template.yml
Outdated
@@ -84,38 +116,39 @@ jobs: | |||
artifact: ${{ parameters.swss_common_artifact_name }} | |||
runVersion: 'latestFromBranch' | |||
runBranch: 'refs/heads/$(BUILD_BRANCH)' | |||
path: $(Build.ArtifactStagingDirectory)/download | |||
allowFailedBuilds: ${{ parameters.allow_failed_artifact }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to allow failed builds here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value for allow_failed_artifact is false. So perhaps that is ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it's set to True in azure-pipeline-dash.yml
below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we are using the branch, dash, of sonic-swss-common which also leverage the sairedis code in Azp: https://dev.azure.com/mssonic/build/_build/results?buildId=166787&view=logs&jobId=61b7eb0d-78fa-5af5-6df0-b6f82d3891a8&j=61b7eb0d-78fa-5af5-6df0-b6f82d3891a8&t=70f86778-6fb4-5097-2f4f-43a5d7a20e70 . But in the repo, sonic-sairedis, there is no related branch named 'dash'. So, there is failed in the sairedis building of sonic-swss-common. If allow_failed_artifact is false (by default), we cannot fetch an available artifact from sonic-swss-common.
I don't think we can directly use the master branch of sonic-swss-common, because other artifacts from DASH like 'syncd' and so on were using a specified commit of sonic-swss-common what was listed in the private sonic-buildimage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per #2501 (comment), we should be using the master branch of swss common. In this case, I don't think we still need to allow failed builds.
/Azp run |
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
.azure-pipelines/build-template.yml
Outdated
@@ -84,38 +116,39 @@ jobs: | |||
artifact: ${{ parameters.swss_common_artifact_name }} | |||
runVersion: 'latestFromBranch' | |||
runBranch: 'refs/heads/$(BUILD_BRANCH)' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the swsscommon dash branch doesn't differ from the master branch - will this change in the future? Should we continue to use master branch here since the swsscommon job keeps failing due to the sairedis repo not having a dash branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that swsscommon for dash will always be on master branch. @prsunny to confirm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed with @prsunny and @prabhataravind, we should always be using the master branch of swsscommon - can you add a new parameter like swss_common_branch_name
so we can specify this for DASh jobs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
/Azp run |
Azure Pipelines successfully started running 2 pipeline(s). |
a57e9f7
to
2fbd14a
Compare
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
azure-pipelines-dash.yml
Outdated
arch: amd64 | ||
sonic_slave: sonic-slave-buster | ||
swss_common_artifact_name: sonic-swss-common | ||
swss_common_branch: dash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change to master branch as per #2501 (comment)
azure-pipelines-dash.yml
Outdated
log_artifact_name: log | ||
gcov_artifact_name: sonic-gcov | ||
sonic_slave: sonic-slave-buster | ||
swss_common_branch: dash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change to master branch as per #2501 (comment)
.azure-pipelines/build-template.yml
Outdated
@@ -84,38 +116,39 @@ jobs: | |||
artifact: ${{ parameters.swss_common_artifact_name }} | |||
runVersion: 'latestFromBranch' | |||
runBranch: 'refs/heads/$(BUILD_BRANCH)' | |||
path: $(Build.ArtifactStagingDirectory)/download | |||
allowFailedBuilds: ${{ parameters.allow_failed_artifact }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per #2501 (comment), we should be using the master branch of swss common. In this case, I don't think we still need to allow failed builds.
@@ -43,18 +83,27 @@ jobs: | |||
pipeline: Azure.sonic-swss-common | |||
artifact: ${{ parameters.swss_common_artifact_name }} | |||
runVersion: 'latestFromBranch' | |||
runBranch: 'refs/heads/$(BUILD_BRANCH)' | |||
runBranch: 'refs/heads/${{ parameters.swss_common_branch }}' | |||
allowFailedBuilds: ${{ parameters.allow_failed_artifact }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're using the master branch of swss common, please remove this
Signed-off-by: Ze Gan <ganze718@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
What I did Add Azp job for DASH. Why I did it DASH needs CLI. How I verified it Check Azp. Signed-off-by: Ze Gan <ganze718@gmail.com>
What I did Add Azp job for DASH. Why I did it DASH needs CLI. How I verified it Check Azp. Signed-off-by: Ze Gan <ganze718@gmail.com>
What I did Add Azp job for DASH. Why I did it DASH needs CLI. How I verified it Check Azp. Signed-off-by: Ze Gan <ganze718@gmail.com>
What I did Add Azp job for DASH. Why I did it DASH needs CLI. How I verified it Check Azp. Signed-off-by: Ze Gan <ganze718@gmail.com>
What I did Add Azp job for DASH. Why I did it DASH needs CLI. How I verified it Check Azp. Signed-off-by: Ze Gan <ganze718@gmail.com>
What I did Add Azp job for DASH. Why I did it DASH needs CLI. How I verified it Check Azp. Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan ganze718@gmail.com
What I did
Add Azp job for DASH.
Why I did it
DASH needs CLI.
How I verified it
Check Azp.
Details if related