Merge pull request ceph#63714 from Naveenaidu/wip-naveen-update-config-diff-script

.github/workflow/diff-ceph-config.yml: only detect the configuration changes that were made in the PR

Author: Naveenaidu

.github/workflows/diff-ceph-config.yml

@@ -20,12 +20,30 @@ jobs:
       - name: checkout ceph.git
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 #v4.2.2
         with:
+          ref: ${{ github.event.pull_request.head.sha }}
           path: ceph
           sparse-checkout: |
             src/script
             src/common/options
             .github/workflows
 
+      - name:  'Get common ancestor between PR and ceph upstream main branch'
+        id: get_common_ancestor
+        env:
+          branch_pr: origin/${{ github.event.pull_request.head.ref }}
+          refspec_pr: +${{ github.event.pull_request.head.sha }}:remotes/origin/${{ github.event.pull_request.head.ref }}
+        working-directory: ceph
+        run: |
+          # Fetch enough history to find a common ancestor commit (aka merge-base):
+          git fetch origin ${{ env.refspec_pr }} --depth=$(( ${{ github.event.pull_request.commits }} + 1 )) \
+            --no-tags --prune --no-recurse-submodules
+
+          # This should get the oldest commit in the local fetched history (the commit in ceph upstream from which PR branched from):
+          COMMON_ANCESTOR=$( git rev-list --first-parent --max-parents=0 --max-count=1 ${{ env.branch_pr }} )
+          COMMON_ANCESTOR_SHA=$( git log --format=%H "${COMMON_ANCESTOR}" )
+
+          echo "COMMON_ANCESTOR_SHA=${COMMON_ANCESTOR_SHA}" >> $GITHUB_ENV
+
       - name: Setup Python
         uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 #v5.6.0
         with:
@@ -41,12 +59,14 @@ jobs:
         env:
           REF_REPO: ${{ github.event.pull_request.base.repo.clone_url }}
           REF_BRANCH: ${{ github.event.pull_request.base.ref }}
+          REF_COMMIT_SHA: ${{ env.COMMON_ANCESTOR_SHA }}
           REMOTE_REPO: ${{ github.event.pull_request.head.repo.clone_url }}
           REMOTE_BRANCH: ${{ github.event.pull_request.head.ref }}
+          REMOTE_COMMIT_SHA: ${{ github.event.pull_request.head.sha }}
         run:  |
           {
             echo 'DIFF_JSON<<EOF'
-            python3 ./src/script/config-diff/config_diff.py diff-branch-remote-repo --ref-branch $REF_BRANCH  --remote-repo $REMOTE_REPO --cmp-branch $REMOTE_BRANCH --format=posix-diff --skip-clone
+            python3 ./src/script/config-diff/config_diff.py diff-branch-remote-repo --ref-branch $REF_BRANCH --ref-commit-sha $REF_COMMIT_SHA --remote-repo $REMOTE_REPO --cmp-branch $REMOTE_BRANCH --cmp-commit-sha $REMOTE_COMMIT_SHA --format=posix-diff --skip-clone
             echo EOF
           } >> "$GITHUB_OUTPUT"
         working-directory: ceph

.github/workflows/scripts/config-diff-post-comment.js

@@ -1,11 +1,37 @@
 module.exports = async ({ github, context, core, configDiff }) => {
   try {
-      // Do not create comment if there are no configuration changes
-      if (!configDiff) {
-        console.log("No changes detected. Skipping comment creation.");
-        return;
+    const { owner, repo } = context.repo;
+    const issueNumber = context.payload.pull_request.number;
+
+    // List all the comments
+    const comments = await github.paginate(
+      github.rest.issues.listComments, {
+        owner,
+        repo,
+        issue_number: issueNumber,
+        per_page: 100,
+      }
+    );
+
+    const existingComment = comments.find(comment => comment.body.includes("### Config Diff Tool Output"));
+
+    // Do not create comment if there are no configuration changes
+    if (!configDiff) {
+      core.info("No changes detected. Skipping comment creation.");
+
+      if (existingComment){
+        // Remove any outdated configuration diff comments
+        core.info("Existing config diff comment found. Deleting it...");
+        await github.rest.issues.deleteComment({
+            comment_id: existingComment.id,
+            owner,
+            repo,
+        });
       }
 
+      return;
+    }
+
       const commentBody = `
 ### Config Diff Tool Output
 
@@ -19,21 +45,15 @@ ${configDiff}
 The above configuration changes are found in the PR. Please update the relevant release documentation if necessary.
   `;
 
-      core.summary.addRaw(commentBody);
-      await core.summary.write()
-
-      const { owner, repo } = context.repo;
-      const issueNumber = context.payload.pull_request.number;
-
       // List all files in the pull request
       core.info("Fetching list of files changed in the pull request...");
       const files = await github.paginate(
           github.rest.pulls.listFiles,
           {
-              owner,
-              repo,
-              pull_number: issueNumber,
-              per_page: 100,
+            owner,
+            repo,
+            pull_number: issueNumber,
+            per_page: 100,
           }
       );
 
@@ -42,33 +62,35 @@ The above configuration changes are found in the PR. Please update the relevant
         // Only annotate the `yaml.in` files present in `src/common/options` folder
         if (file.filename.endsWith(".yaml.in") && file.filename.startsWith("src/common/options/")) {
             core.info(`Annotating file: ${file.filename}`);
+            // Show annotations only at the start of the file
             core.notice(
-                `Configuration changes detected in ${file.filename}. Please update the relevant release documentation if necessary.`,
+                `Configuration changes detected in ${file.filename}. Please update the release documentation if necessary.`,
                 {
-                    title: "Configuration Change Detected",
-                    file: file.filename,
-                    startLine: 1,
-                    endLine: 1,
+                  title: "Configuration Change Detected",
+                  file: file.filename,
+                  startLine: 1,
+                  endLine: 1,
                 }
             );
         }
-    });
+      });
 
-      
-      // List all the comments
-      const comments = await github.paginate(
-        github.rest.issues.listComments, {
+      core.summary.addRaw(commentBody);
+      await core.summary.write()
+
+      if (existingComment) {
+        // There might have been new configuration changes made after posting
+        // the first comment. Hence replace the old comment with the new updated
+        // changes
+        core.info("A config diff comment already exists, updating it...");
+
+        // Update the existing comment
+        await github.rest.issues.updateComment ({
+          comment_id: existingComment.id,
           owner,
           repo,
-          issue_number: issueNumber,
-          per_page: 100,
-        }
-      );
-  
-      const existingComment = comments.find(comment => comment.body.includes("### Config Diff Tool Output"));
-  
-      if (existingComment) {
-        core.info("A config diff comment already exists, deleting it...");
+          body: commentBody,
+        });
       } else {
         core.info("Creating a new config diff comment...");
         // Create a new comment
@@ -80,7 +102,7 @@ The above configuration changes are found in the PR. Please update the relevant
         });
 
       }
-  
+
       // Set the status as FAILED if any configuration changes are detected
       core.setFailed("Configuration Changes Detected, Update release documents - if necessary");
     } catch (error) {

src/script/config-diff/README.md

@@ -52,6 +52,8 @@ python3 config_diff.py <mode> [options]
     - `--cmp-branch`: The branch to compare.
     - `--remote-repo`: The remote repository URL for the branch to compare.
     - `--ref-repo`: (Optional) The repository URL for the reference branch. Defaults to the Ceph upstream repository.
+    - `--ref-commit-sha`: (Optional) The commit sha for the reference branch that is used for reference
+    - `--cmp-commit-sha`: (Optional) The commit sha of the comparing branch
     - `--skip-clone`: (Optional) Skips cloning repositories for diff. **Note**: When using this flag, the script must be run from a valid Ceph upstream repository or a forked repository that has access to the branches present in the upstream repository or already contains those branches.
     - `--format`: (Optional) Specify the output format for the configuration diff. Options are `json` or `posix-diff`. Default is `json`.