Fix test failures and code cleanup

Fixed these tests from randomly failing:
 - locking_norecord.js
 - simple_plain_cs_mysql_comm_stack_norecord.js

Also, prevent the error message "Instance 'foo' is already part of this
InnoDB Cluster but is (MISSING).", introduced when fixing #30896233,
from showing in dba.rebootClusterFromCompleteOutage().

Change-Id: Ib83d02e447125c863b12a5878779fe92d257a656

Author: joaosigma

modules/adminapi/common/instance_validations.cc

@@ -31,13 +31,11 @@
 #include "modules/adminapi/common/dba_errors.h"
 #include "modules/adminapi/common/metadata_storage.h"
 #include "modules/adminapi/common/provision.h"
-#include "modules/adminapi/common/sql.h"
-#include "modules/adminapi/mod_dba.h"
 #include "mysqlshdk/include/shellcore/console.h"
-#include "mysqlshdk/libs/config/config_server_handler.h"
 #include "mysqlshdk/libs/mysql/replication.h"
 #include "mysqlshdk/libs/textui/textui.h"
 #include "mysqlshdk/libs/utils/utils_net.h"
+#include "mysqlshdk/libs/utils/utils_string.h"
 #include "mysqlshdk/libs/utils/version.h"
 
 // Naming convention for validations:
@@ -530,7 +528,7 @@ void validate_performance_schema_enabled(
 void ensure_instance_not_belong_to_cluster(
     const std::shared_ptr<Instance> &instance,
     const std::shared_ptr<Instance> &cluster_instance,
-    const std::string &cluster_id) {
+    std::string_view cluster_id, bool omit_missing_instance_warn) {
   auto metadata = std::make_shared<MetadataStorage>(instance);
   auto type = mysqlsh::dba::get_instance_type(*metadata, *instance);
 
@@ -553,10 +551,12 @@ void ensure_instance_not_belong_to_cluster(
 
       metadata_cluster->get_instance_by_uuid(instance->get_uuid());
 
-      current_console()->print_error(
-          "Instance '" + instance->descr() +
-          "' is already part of this InnoDB Cluster but is (MISSING). Please "
-          "use <Cluster>.<<<rejoinInstance()>>> to rejoin it.");
+      if (!omit_missing_instance_warn)
+        current_console()->print_error(
+            shcore::str_format("Instance '%s' is already part of this InnoDB "
+                               "Cluster but is (MISSING). Please use "
+                               "<Cluster>.<<<rejoinInstance()>>> to rejoin it.",
+                               instance->descr().c_str()));
 
       throw shcore::Exception("The instance '" + instance->descr() +
                                   "' is already part of this InnoDB Cluster",

modules/adminapi/common/instance_validations.h

@@ -65,7 +65,7 @@ void validate_performance_schema_enabled(
 void ensure_instance_not_belong_to_cluster(
     const std::shared_ptr<Instance> &instance,
     const std::shared_ptr<Instance> &cluster_instance,
-    const std::string &cluster_id);
+    std::string_view cluster_id, bool omit_missing_instance_warn = false);
 
 void ensure_instance_not_belong_to_metadata(
     const mysqlshdk::mysql::IInstance &instance,

modules/adminapi/dba/reboot_cluster_from_complete_outage.cc

@@ -35,13 +35,10 @@
 #include "modules/adminapi/common/sql.h"
 #include "modules/adminapi/common/topology_executor.h"
 #include "modules/adminapi/common/validations.h"
-#include "modules/adminapi/mod_dba.h"
 #include "modules/errors.h"
-#include "mysqlshdk/include/shellcore/utils_help.h"
 #include "mysqlshdk/libs/mysql/async_replication.h"
 #include "mysqlshdk/libs/utils/utils_net.h"
 #include "mysqlshdk/libs/utils/utils_string.h"
-#include "mysqlshdk/shellcore/shell_console.h"
 
 namespace mysqlsh::dba {
 
@@ -734,7 +731,7 @@ void Reboot_cluster_from_complete_outage::reboot_seed(
     try {
       mysqlsh::dba::checks::ensure_instance_not_belong_to_cluster(
           m_target_instance, m_cluster->impl()->get_cluster_server(),
-          m_cluster->impl()->get_id());
+          m_cluster->impl()->get_id(), true);
     } catch (const shcore::Exception &exp) {
       m_already_member =
           (exp.code() == SHERR_DBA_ASYNC_MEMBER_INCONSISTENT) ||

unittest/scripts/auto/js_adminapi/scripts/locking_norecord.js

@@ -192,12 +192,11 @@ cluster_lock_check(function() {
 
 cluster.addInstance(__sandbox_uri3, {ipAllowlist: allowlist});
 
-disable_auto_rejoin(__mysql_sandbox_port3);
+cluster.status();
 
-shell.connect(__sandbox_uri1);
-testutil.killSandbox(__mysql_sandbox_port3);
-testutil.waitMemberState(__mysql_sandbox_port3, "(MISSING)");
-testutil.startSandbox(__mysql_sandbox_port3);
+var session3 = mysql.getSession(__sandbox_uri3);
+session3.runSql("STOP group_replication;");
+session3.close();
 
 cluster_lock_check(function() {
     cluster.rejoinInstance(__sandbox_uri3, {ipAllowlist: allowlist});
@@ -320,22 +319,20 @@ instance_lock_check(session3, __mysql_sandbox_port3, function() {
     dba.configureLocalInstance(__sandbox_uri3);
 });
 
-//@<> exclusive instance lock on dba.rebootClusterFromCompleteOutage()
-
 session1.close();
 session3.close();
+session4.close();
 
-disable_auto_rejoin(__mysql_sandbox_port1);
-disable_auto_rejoin(__mysql_sandbox_port2);
+//@<> exclusive instance lock on dba.rebootClusterFromCompleteOutage()
 
-testutil.killSandbox(__mysql_sandbox_port1);
-testutil.killSandbox(__mysql_sandbox_port2);
-testutil.startSandbox(__mysql_sandbox_port1);
-testutil.startSandbox(__mysql_sandbox_port2);
+cluster.status();
 
 session1 = mysql.getSession(__sandbox_uri1);
 session2 = mysql.getSession(__sandbox_uri2);
 
+session1.runSql("STOP group_replication;");
+session2.runSql("STOP group_replication;");
+
 shell.connect(__sandbox_uri1);
 
 instance_lock_check(session1, __mysql_sandbox_port1, function() {

unittest/scripts/auto/js_adminapi_async/scripts/locking_norecord.js

@@ -267,10 +267,6 @@ testutil.releaseLocks(session1, lock_instance);
 rs.rejoinInstance(__sandbox1);
 EXPECT_OUTPUT_CONTAINS(`The instance '${localhost}:${__mysql_sandbox_port1}' is ONLINE and the primary of the ReplicaSet.`);
 
-//@<> (BUG#30673719) rejoinInstance on primary should not fail due to double self lock (using hostname).
-rs.rejoinInstance(__endpoint1);
-EXPECT_OUTPUT_CONTAINS(`The instance '${localhost}:${__mysql_sandbox_port1}' is ONLINE and the primary of the ReplicaSet.`);
-
 //@<> (BUG#30673719) rejoinInstance on primary should not fail due to double self lock (using hostname_ip).
 var address_using_ip = hostname_ip + ":" + __mysql_sandbox_port1;
 rs.rejoinInstance(address_using_ip);

unittest/scripts/auto/js_adminapi_clusterset/scripts/simple_plain_cs_mysql_comm_stack_norecord.js

@@ -310,13 +310,16 @@ EXPECT_NO_THROWS(function() { cluster.removeInstance(__sandbox_uri3); });
 CHECK_PRIMARY_CLUSTER([__sandbox_uri1, __sandbox_uri2], cluster)
 
 //@<> addInstance on replica cluster
+shell.connect(__sandbox_uri4);
+
 EXPECT_NO_THROWS(function() { replicacluster.addInstance(__sandbox_uri3, {recoveryMethod: "clone"}); });
+testutil.waitMemberState(__mysql_sandbox_port3, "ONLINE");
 CHECK_REPLICA_CLUSTER([__sandbox_uri4, __sandbox_uri3], cluster, replicacluster);
 
 //@<> rejoinInstance on a replica cluster
 session3 = mysql.getSession(__sandbox_uri3);
 session3.runSql("STOP group_replication");
-shell.connect(__sandbox_uri4);
+
 EXPECT_NO_THROWS(function() { replicacluster.rejoinInstance(__sandbox_uri3); });
 testutil.waitMemberState(__mysql_sandbox_port3, "ONLINE");
 CHECK_REPLICA_CLUSTER([__sandbox_uri4, __sandbox_uri3], cluster, replicacluster);