fix(opensearch): run the full reindex after an absorbed OS halt (#36222)

.github/workflows/issue_comp_link-issue-to-pr.yml

@@ -498,8 +498,45 @@ jobs:
 
           current_body=$(echo "$current_pr" | jq -r '.body // ""')
 
-          # Check if issue reference already exists (word-boundary match so #10 does not match #100)
-          if ! echo "$current_body" | grep -qE "(^|[^0-9])#${issue_number}([^0-9]|\$)"; then
+          # Decide whether the PR already *closes* this issue, not merely whether the
+          # number appears somewhere. The previous bare "#N" test treated an incidental
+          # prose mention ("reproducing QA's report on #36222") as an existing link and
+          # skipped the patch below, so the PR carried no closing reference at all and
+          # the issue would not close on merge (#36933).
+          already_linked=false
+
+          # GitHub's own parser is authoritative and also catches markdown-linked refs
+          # ("fixes [#123](url)") that a regex misses.
+          owner="${GITHUB_REPOSITORY%/*}"
+          repo="${GITHUB_REPOSITORY#*/}"
+          gh_err=$(mktemp)
+          if closing_issues=$(gh api graphql \
+              -F owner="$owner" \
+              -F name="$repo" \
+              -F num="$pr_number" \
+              -f query='query($owner:String!,$name:String!,$num:Int!){repository(owner:$owner,name:$name){pullRequest(number:$num){closingIssuesReferences(first:20){nodes{number}}}}}' \
+              --jq '.data.repository.pullRequest.closingIssuesReferences.nodes[].number' 2>"$gh_err"); then
+            if echo "$closing_issues" | grep -qx "$issue_number"; then
+              already_linked=true
+            fi
+          else
+            echo "::warning::closingIssuesReferences lookup failed; relying on keyword matching alone:"
+            cat "$gh_err"
+          fi
+          rm -f "$gh_err"
+
+          # Keyword match as well as — not instead of — the API: it covers the window
+          # right after our own PATCH, where GraphQL may not yet report the new
+          # reference and a second append would duplicate the line. Accepts the same
+          # forms as the link detection above: "#N", "owner/repo#N", full issue URL.
+          # Word-boundary on the number so #10 does not match #100.
+          closing_kw='(close[ds]?|fix(e[ds])?|resolve[ds]?)(:)?[[:space:]]+'
+          if echo "$current_body" | grep -qiE "${closing_kw}([a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+)?#${issue_number}([^0-9]|\$)" \
+            || echo "$current_body" | grep -qiE "${closing_kw}https://github\.com/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/issues/${issue_number}([^0-9]|\$)"; then
+            already_linked=true
+          fi
+
+          if [[ "$already_linked" != 'true' ]]; then
             # Add issue reference to PR body
             if [[ -n "$current_body" && "$current_body" != "null" ]]; then
               new_body=$(printf "%s\n\nThis PR fixes: #%s" "$current_body" "$issue_number")
@@ -517,7 +554,7 @@ jobs:
 
             echo "Added issue #$issue_number reference to PR #$pr_number body"
           else
-            echo "Issue #$issue_number already referenced in PR #$pr_number body"
+            echo "Issue #$issue_number is already a closing reference on PR #$pr_number"
           fi
 
       - name: Add failure comment to PR

dotCMS/src/main/java/com/dotcms/content/elasticsearch/business/ContentletIndexAPIImpl.java

@@ -25,6 +25,7 @@
 import com.dotcms.content.index.IndexAPI;
 import com.dotcms.content.index.IndexAPIImpl;
 import com.dotcms.content.index.IndexConfigHelper.MigrationPhase;
+import com.dotcms.content.index.MigrationHaltReport;
 import com.dotcms.content.index.opensearch.IndexStartupValidator;
 import com.dotcms.content.index.opensearch.OSIndexAPIImpl;
 import com.dotcms.content.index.opensearch.OSIndexAPIImpl.ConnectionFailureKind;
@@ -1175,6 +1176,13 @@ boolean handleOsBootstrapFailure(final String workingName, final String liveName
                 + " Any OS index created before the failure is left in the cluster and is reused or"
                 + " repaired by the next bootstrap; it is never registered in the OS index store."
                 + " Fix the cause above and re-enable the migration phase when ready.", e);
+
+        // Keep the classified cause where callers outside the index layer can read it: the log line
+        // above is for support, this is what the operator who triggered the operation gets told.
+        MigrationHaltReport.record(new MigrationHaltReport(phase.name(), kind.name(),
+                kind.remediation(),
+                operationsOS.toPhysicalName(workingName) + ", "
+                        + operationsOS.toPhysicalName(liveName)));
         haltMigration();
 
         // haltMigration() writes the phase through Config, which the DB-backed ConfigSystemTable
@@ -1341,28 +1349,62 @@ public boolean reindexSwitchover(boolean forceSwitch) throws DotDataException {
     @WrapInTransaction
     public synchronized IndexStartResult fullReindexStart() throws DotDataException {
         if (indexReady() && !isInFullReindex()) {
-            final User currentUser = Try.of(
-                    () -> PortalUtil.getUser(HttpServletRequestThreadLocal.INSTANCE.getRequest()))
-                    .getOrNull();
-            if (currentUser != null) {
-                Logger.info(this, "Full reindex started by user: "
-                        + currentUser.getUserId() + " (" + currentUser.getEmailAddress()
-                        + ") at " + new java.util.Date());
-            } else {
-                Logger.info(this, "Full reindex started by system user at " + new java.util.Date());
-            }
+            return startFullReindex();
+        }
 
-            final String ts = ContentletIndexAPI.threadSafeTimestampFormatter
-                    .format(LocalDateTime.now());
-            initAndPointReindex(ts);
+        final boolean migrationWasRunning = isMigrationStarted();
+        final IndexStartResult bootstrapResult = initIndex();
 
-            return ImmutableIndexStartResult.builder()
-                    .indexSuffixES(ts)
-                    .indexSuffixOS(isMigrationNotStarted() ? "" : ts)
-                    .build();
+        // A dual-phase OpenSearch rejection makes indexReady() false — an index the OS user may not
+        // even probe reads as missing — so the branch above is skipped and the caller silently gets
+        // a bootstrap instead of the full reindex it asked for. initIndex() absorbs that rejection
+        // and halts the migration (handleOsBootstrapFailure), and once dotCMS is ES-only the reindex
+        // is possible again: run it rather than return a downgraded no-op (issue #36222).
+        //
+        // Gated on the halt, not on a bare indexReady() re-check: on a fresh install initIndex()
+        // legitimately turns indexReady() true by creating the indices, and reindexing brand-new
+        // empty indices is not this method's contract.
+        if (migrationWasRunning && !isMigrationStarted() && indexReady() && !isInFullReindex()) {
+            Logger.warn(this, "The OpenSearch migration was halted while preparing the full reindex."
+                    + " Continuing on Elasticsearch only — see the ERROR above for the cause."
+                    + MigrationHaltReport.last()
+                            .map(report -> " " + report.operatorMessage()).orElse(""));
+            return startFullReindex();
+        }
+
+        return bootstrapResult;
+    }
+
+    /**
+     * Creates the reindex slots for every applicable provider and reports the timestamp they share.
+     *
+     * <p>Extracted from {@link #fullReindexStart()} so that the same work runs both on the happy
+     * path and on the retry that follows an absorbed OpenSearch failure (issue #36222). Callers must
+     * have checked {@link #indexReady()} and {@link #isInFullReindex()} first.</p>
+     *
+     * @return the ES suffix always, and the OS suffix only while the migration is running
+     * @throws DotDataException on persistence or creation failure
+     */
+    private IndexStartResult startFullReindex() throws DotDataException {
+        final User currentUser = Try.of(
+                () -> PortalUtil.getUser(HttpServletRequestThreadLocal.INSTANCE.getRequest()))
+                .getOrNull();
+        if (currentUser != null) {
+            Logger.info(this, "Full reindex started by user: "
+                    + currentUser.getUserId() + " (" + currentUser.getEmailAddress()
+                    + ") at " + new java.util.Date());
         } else {
-            return initIndex();
+            Logger.info(this, "Full reindex started by system user at " + new java.util.Date());
         }
+
+        final String ts = ContentletIndexAPI.threadSafeTimestampFormatter
+                .format(LocalDateTime.now());
+        initAndPointReindex(ts);
+
+        return ImmutableIndexStartResult.builder()
+                .indexSuffixES(ts)
+                .indexSuffixOS(isMigrationNotStarted() ? "" : ts)
+                .build();
     }
 
     /**

dotCMS/src/main/java/com/dotcms/content/index/MigrationHaltReport.java

@@ -0,0 +1,62 @@
+package com.dotcms.content.index;
+
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * Why the ES → OpenSearch migration last switched itself off.
+ *
+ * <p>When OpenSearch rejects an index operation while it is only the shadow store, the failure is
+ * absorbed and the migration is halted so that dotCMS keeps serving from Elasticsearch
+ * (issue #36222). That policy is right, but on its own it is silent: the reason lives in the log and
+ * nothing reaches the operator who pressed the button. This carrier keeps the classified cause and
+ * its remediation so callers outside the index layer — the reindex REST resource, which carries no
+ * vendor imports — can report <em>why</em> the migration stopped, not merely <em>that</em> it did.</p>
+ *
+ * <p><strong>Scope:</strong> the last report is held per JVM, deliberately matching the scope of the
+ * halt it describes: {@link IndexConfigHelper.MigrationPhase#reset()} is a runtime-only change, so a
+ * restart clears both the halted phase and the reason for it. It is not persisted and not
+ * cluster-wide — each node reports the halt it absorbed itself.</p>
+ *
+ * @param haltedPhase the phase that was active when the failure was absorbed
+ * @param cause       classified failure kind, e.g. {@code AUTH_FORBIDDEN}
+ * @param remediation operator-facing description of how to fix that kind of failure
+ * @param indexNames  the physical OpenSearch index names the operation was rejected for
+ */
+public record MigrationHaltReport(String haltedPhase, String cause, String remediation,
+                                  String indexNames) {
+
+    private static final AtomicReference<MigrationHaltReport> LAST = new AtomicReference<>();
+
+    /**
+     * Records {@code report} as the most recent halt, replacing any previous one.
+     *
+     * @param report the halt to remember
+     */
+    public static void record(final MigrationHaltReport report) {
+        LAST.set(report);
+    }
+
+    /**
+     * Returns the most recent halt absorbed by this JVM, or empty when the migration has not been
+     * halted since start-up.
+     *
+     * @return the last halt report, if any
+     */
+    public static Optional<MigrationHaltReport> last() {
+        return Optional.ofNullable(LAST.get());
+    }
+
+    /**
+     * Returns a single sentence an operator can act on: what happened, that Elasticsearch is still
+     * serving, why it happened, and what to do about it.
+     *
+     * @return the operator-facing message
+     */
+    public String operatorMessage() {
+        return "The OpenSearch migration was switched off (was " + haltedPhase + ") and dotCMS is"
+                + " serving from Elasticsearch only. Cause: " + cause + " (" + remediation + ")."
+                + " Rejected index names: " + indexNames + "."
+                + " Fix the cause and re-enable the migration phase when ready.";
+    }
+}

dotCMS/src/main/java/com/dotcms/rest/api/v1/index/ESIndexResource.java

@@ -8,6 +8,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.ws.rs.DELETE;
@@ -36,13 +37,16 @@
 import com.dotcms.content.elasticsearch.business.IndexType;
 import com.dotcms.content.elasticsearch.business.IndiciesAPI;
 import com.dotcms.content.index.IndexAPI;
+import com.dotcms.content.index.IndexConfigHelper.MigrationPhase;
+import com.dotcms.content.index.MigrationHaltReport;
 import com.dotcms.content.index.domain.NodeStats;
 import com.dotcms.content.elasticsearch.util.ESReindexationProcessStatus;
 import com.dotcms.contenttype.model.type.ContentType;
 import com.google.common.annotations.VisibleForTesting;
 import com.dotcms.rest.ErrorEntity;
 import com.dotcms.rest.InitDataObject;
 import com.dotcms.rest.ResourceResponse;
+import com.dotcms.rest.MessageEntity;
 import com.dotcms.rest.ResponseEntityView;
 import com.dotcms.rest.WebResource;
 import com.dotcms.rest.annotation.NoCache;
@@ -341,6 +345,10 @@ public Response startReindex(@Context final HttpServletRequest request, @Context
         System.setProperty("es.index.number_of_shards", String.valueOf(shards));
         Logger.info(this, "Running Contentlet Reindex");
 
+        // Captured before the call: a shadow-phase OpenSearch failure is absorbed deep in the index
+        // layer, which halts the migration to ES-only without any of it reaching this response.
+        final boolean migrationWasRunning = !MigrationPhase.current().isMigrationNotStarted();
+
         if(!DOTALL.equals(contentType)) {
             ContentType type = APILocator.getContentTypeAPI(APILocator.systemUser()).find(contentType);
             Logger.info(this.getClass(), "Starting reindex of " + type.name());
@@ -351,9 +359,39 @@ public Response startReindex(@Context final HttpServletRequest request, @Context
             APILocator.getContentletAPI().refreshAllContent();
         }
 
+        final Optional<String> haltMessage = migrationHaltMessage(migrationWasRunning);
+        if (haltMessage.isPresent()) {
+            Logger.warn(this.getClass(), haltMessage.get());
+            return Response.ok(new ResponseEntityView<>(
+                    ESReindexationProcessStatus.getProcessIndexationMap(),
+                    List.of(new MessageEntity(haltMessage.get())))).build();
+        }
+
         return getReindexationProgress(request, response);
 
     }
+
+    /**
+     * Returns what to tell the operator when the OpenSearch migration switched itself off while this
+     * request was running, or empty when it did not (issue #36222).
+     *
+     * <p>The reindex itself succeeds on Elasticsearch — that degradation is deliberate — but it must
+     * not be silent: without this the response is indistinguishable from a run where nothing went
+     * wrong, and the only trace is a log line nobody is watching.</p>
+     *
+     * @param migrationWasRunning whether the migration was active before the reindex was triggered
+     * @return the operator-facing message, if the migration was halted during this request
+     */
+    private static Optional<String> migrationHaltMessage(final boolean migrationWasRunning) {
+        if (!migrationWasRunning || !MigrationPhase.current().isMigrationNotStarted()) {
+            return Optional.empty();
+        }
+        return Optional.of(MigrationHaltReport.last()
+                .map(MigrationHaltReport::operatorMessage)
+                .orElse("The OpenSearch migration was switched off while this reindex was starting"
+                        + " and dotCMS is serving from Elasticsearch only. See the log for the"
+                        + " cause, and re-enable the migration phase once it is fixed."));
+    }
 
     @CloseDBIfOpened
     @DELETE