From dbff3a09a8f75afe2a6db1e0cffe1b69fdaa2f9e Mon Sep 17 00:00:00 2001
From: Marcus Gartner <marcus@cockroachlabs.com>
Date: Thu, 26 Oct 2023 10:57:39 -0400
Subject: [PATCH] sql/logictest: skip unreliable mixed version tests

Clusters do not reliably complete upgrades in mixed version tests in CI.
This commit skips test cases that rely on completing the upgrade to
avoid flakes.

Informs #112621
---
 .../logic_test/mixed_version_procedure        | 30 +++++-----
 .../mixed_version_udf_execute_privileges      | 58 ++++++++++---------
 .../logic_test/mixed_version_udf_mutations    | 48 +++++++--------
 3 files changed, 71 insertions(+), 65 deletions(-)

diff --git a/pkg/sql/logictest/testdata/logic_test/mixed_version_procedure b/pkg/sql/logictest/testdata/logic_test/mixed_version_procedure
index 495df4488b2c..5a9d7e3fd253 100644
--- a/pkg/sql/logictest/testdata/logic_test/mixed_version_procedure
+++ b/pkg/sql/logictest/testdata/logic_test/mixed_version_procedure
@@ -33,17 +33,19 @@ CREATE PROCEDURE p() LANGUAGE SQL AS 'SELECT 1'
 
 # Upgrade all nodes.
 
-upgrade 1
-
-upgrade 2
-
-# Makes sure the upgrade job has finished, and the cluster version gate is
-# passed.
-query B retry
-SELECT crdb_internal.is_at_least_version('23.2')
-----
-true
-
-# Creating a procedure should now be possible.
-statement ok
-CREATE PROCEDURE p() LANGUAGE SQL AS 'SELECT 1'
+# TODO(#112621): Unskip these tests. It is currently skipped because the cluster
+# does not reliably complete the upgrade in CI.
+# upgrade 1
+#
+# upgrade 2
+#
+# # Makes sure the upgrade job has finished, and the cluster version gate is
+# # passed.
+# query B retry
+# SELECT crdb_internal.is_at_least_version('23.2')
+# ----
+# true
+#
+# # Creating a procedure should now be possible.
+# statement ok
+# CREATE PROCEDURE p() LANGUAGE SQL AS 'SELECT 1'
diff --git a/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_execute_privileges b/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_execute_privileges
index a15b3721621d..9673688e371f 100644
--- a/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_execute_privileges
+++ b/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_execute_privileges
@@ -71,31 +71,33 @@ true
 
 # Makes sure the upgrade job has finished, and the cluster version gate is
 # passed.
-query B retry
-SELECT crdb_internal.is_at_least_version('23.2')
-----
-true
-
-query TTTTTTB colnames
-SELECT * FROM [SHOW GRANTS ON FUNCTION f] ORDER BY grantee
-----
-database_name  schema_name  routine_id  routine_signature  grantee  privilege_type  is_grantable
-test           public       100106      f()                admin    ALL             true
-test           public       100106      f()                public   EXECUTE         false
-test           public       100106      f()                root     ALL             true
-
-user testuser1 nodeidx=1
-
-# Makes sure the user can execute f on node 1.
-query I
-SELECT f()
-----
-1
-
-user testuser1 nodeidx=2
-
-# Makes sure the user can execute f on node 2.
-query I
-SELECT f()
-----
-1
+# TODO(#112621): Unskip these tests. It is currently skipped because the cluster
+# does not reliably complete the upgrade in CI.
+# query B retry
+# SELECT crdb_internal.is_at_least_version('23.2')
+# ----
+# true
+#
+# query TTTTTTB colnames
+# SELECT * FROM [SHOW GRANTS ON FUNCTION f] ORDER BY grantee
+# ----
+# database_name  schema_name  routine_id  routine_signature  grantee  privilege_type  is_grantable
+# test           public       100106      f()                admin    ALL             true
+# test           public       100106      f()                public   EXECUTE         false
+# test           public       100106      f()                root     ALL             true
+#
+# user testuser1 nodeidx=1
+#
+# # Makes sure the user can execute f on node 1.
+# query I
+# SELECT f()
+# ----
+# 1
+#
+# user testuser1 nodeidx=2
+#
+# # Makes sure the user can execute f on node 2.
+# query I
+# SELECT f()
+# ----
+# 1
diff --git a/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_mutations b/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_mutations
index 98380b30724b..9f39b97b060c 100644
--- a/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_mutations
+++ b/pkg/sql/logictest/testdata/logic_test/mixed_version_udf_mutations
@@ -83,26 +83,28 @@ $$ LANGUAGE SQL;
 # Upgrade all nodes and test that creating UDFs with mutations succeeds.
 # ----------------------------------------------------------------------
 
-upgrade 1
-
-upgrade 2
-
-query B retry
-SELECT crdb_internal.is_at_least_version('23.2')
-----
-true
-
-statement ok
-CREATE FUNCTION f_insert() RETURNS VOID AS $$
-  INSERT INTO t VALUES (1,1);
-$$ LANGUAGE SQL;
-
-statement ok
-CREATE FUNCTION f_delete() RETURNS VOID AS $$
-  DELETE FROM t WHERE a = 1;
-$$ LANGUAGE SQL;
-
-statement ok
-CREATE FUNCTION f_update() RETURNS VOID AS $$
-  UPDATE t SET b = 1 WHERE a = 1;
-$$ LANGUAGE SQL;
+# TODO(#112621): Unskip these tests. It is currently skipped because the cluster
+# does not reliably complete the upgrade in CI.
+# upgrade 1
+#
+# upgrade 2
+#
+# query B retry
+# SELECT crdb_internal.is_at_least_version('23.2')
+# ----
+# true
+#
+# statement ok
+# CREATE FUNCTION f_insert() RETURNS VOID AS $$
+#   INSERT INTO t VALUES (1,1);
+# $$ LANGUAGE SQL;
+#
+# statement ok
+# CREATE FUNCTION f_delete() RETURNS VOID AS $$
+#   DELETE FROM t WHERE a = 1;
+# $$ LANGUAGE SQL;
+#
+# statement ok
+# CREATE FUNCTION f_update() RETURNS VOID AS $$
+#   UPDATE t SET b = 1 WHERE a = 1;
+# $$ LANGUAGE SQL;