Merge ~aym-frikha/charm-mongodb:branch_1946810 into charm-mongodb:master

[Aymen Frikha]

resolve bug: #1946810 that prevent the MongoDB replication.
resolve bug: #1952908 That make the CI failing

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:04743b29c4206cdfd73119b7069c6153d8289222
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/24/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/235/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/682/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/24//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:75a40fed2ac570c2a9d6dc3d979c5206bcf2395d
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/25/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/236/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/683/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/25//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:11938d24cc19fd6e5a14ef36796054be19fdbb28
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/26/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/237/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/684/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/26//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:11938d24cc19fd6e5a14ef36796054be19fdbb28
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/27/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/240/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/686/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/27//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:11938d24cc19fd6e5a14ef36796054be19fdbb28
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/28/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/241/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/687/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/28//rebuild

review: Needs Fixing (continuous-integration)

[Xav Paice (xavpaice)]

I note that not every reference to default_mongodb_* is prefixed with a conditional for is_percona_source(). Is that deliberate because it's not needed in some cases, or is it maybe easier to have is_percona_source used in some initial variable setup outside of the remaining functions, so that we have the `# Global variables ` section set up with generic names for paths that are adjusted according to the package source?

Regards the CI failures, I'm running a test to see if it's due to Xenial and various Python package changes that have recently meant for some other charms that we've had to remove Xenial support.

review: Needs Information

[Xav Paice (xavpaice)]

Functests passed for me after the following patch, I'd support removing Xenial from this release.

diff --git a/metadata.yaml b/metadata.yaml
index 4604c93..a4218ea 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -20,7 +20,6 @@ tags:
 series:
     - focal
     - bionic
- - xenial
 provides:
   nrpe-external-master:
     interface: nrpe-external-master
diff --git a/tests/functional/tests/bundles/xenial.yaml b/tests/functional/tests/bundles/xenial.yaml
deleted file mode 100644
index 56c5081..0000000
--- a/tests/functional/tests/bundles/xenial.yaml
+++ /dev/null
@@ -1,9 +0,0 @@
-series: xenial
-description: "mongodb-charm test bundle"
-applications:
- mongodb:
- num_units: 3
- constraints: root-disk=10G
- options:
- replicaset: testset
- backup_directory: /var/backups
diff --git a/tests/functional/tests/tests.yaml b/tests/functional/tests/tests.yaml
index 72347b4..cb8611c 100644
--- a/tests/functional/tests/tests.yaml
+++ b/tests/functional/tests/tests.yaml
@@ -1,9 +1,5 @@
 charm_name: mongodb
 tests:
- - model_alias_xenial:
- - tests.tests_mongodb.BasicMongodbCharmTest
- - tests.tests_mongodb.ReplicatedMongodbCharmTest
- - tests.tests_mongodb.XenialMongodbCharmTest
   - model_alias_bionic:
       - tests.tests_mongodb.BasicMongodbCharmTest
       - tests.tests_mongodb.CharmOperationTest
@@ -22,7 +18,6 @@ tests:
       - tests.tests_mongodb.ShardedMongodbCharmTest
       - tests.tests_mongodb.CharmOperationTest
 gate_bundles:
- - model_alias_xenial: xenial
   - model_alias_bionic: bionic
   - model_alias_bionic_replicated: bionic-replicated
   - model_alias_focal: focal

[Aymen Frikha (aym-frikha)]

> I note that not every reference to default_mongodb_* is prefixed with a
> conditional for is_percona_source(). Is that deliberate because it's not
> needed in some cases, or is it maybe easier to have is_percona_source used in
> some initial variable setup outside of the remaining functions, so that we
> have the `# Global variables ` section set up with generic names for paths
> that are adjusted according to the package source?

For default_mongodb_config and default_mongodb_pid_file, every reference is prefixed with a conditional for is_percona_source() to be adjusted based on the package source, only the default_mongodb_init_config variable is not prefixed since it's not necessary in this situation.
In my opinion, the charm code needs a big redesign (split into classes and scripts) to have simpler review process and understandable code.

> Regards the CI failures, I'm running a test to see if it's due to Xenial and
> various Python package changes that have recently meant for some other charms
> that we've had to remove Xenial support.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:139336d55a5524f8072594c37db460e613e67b29
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/29/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/246/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/691/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/29//rebuild

review: Needs Fixing (continuous-integration)

[Xav Paice (xavpaice)]

Change LGTM, Unit and Lint pass in CI, functests not but they're OK locally for me.

I'll review the functest issues in CI separately, it could be a tester machine issue (unit tests failed on my local Bionic test runner, but functests pass).

2021-12-07 22:12:03 [INFO] Events:
  Deploy Bundle:
    Start: 1638914731.8535476
    Finish: 1638914737.2969482
    Elapsed Time: 5.443400621414185
    PCT Of Run Time: 2
  Prepare Environment:
    Start: 1638914729.4626167
    Finish: 1638914731.8533437
    Elapsed Time: 2.3907270431518555
    PCT Of Run Time: 1
  Test tests.tests_mongodb.BasicMongodbCharmTest:
    Start: 1638915099.0387628
    Finish: 1638915101.6677506
    Elapsed Time: 2.6289877891540527
    PCT Of Run Time: 1
  Test tests.tests_mongodb.CharmOperationTest:
    Start: 1638915102.7218187
    Finish: 1638915105.89774
    Elapsed Time: 3.1759212017059326
    PCT Of Run Time: 1
  Test tests.tests_mongodb.ReplicatedMongodbCharmTest:
    Start: 1638915101.6679058
    Finish: 1638915102.72168
    Elapsed Time: 1.053774118423462
    PCT Of Run Time: 1
  Wait for Deployment:
    Start: 1638914737.2969673
    Finish: 1638915098.8577418
    Elapsed Time: 361.56077456474304
    PCT Of Run Time: 97
Metadata: {}

____________________________________________________________________________________________________ summary _____________________________________________________________________________________________________
  func: commands succeeded
  congratulations :)

review: Approve

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:139336d55a5524f8072594c37db460e613e67b29
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/30/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/250/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/692/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-mongodb-ci/30//rebuild

review: Approve (continuous-integration)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision f35fe0bb10a95b9fcbf5429a7f10c882bb58fce2

diff --git a/hooks/hooks.py b/hooks/hooks.py
index 42af6e3..9d07683 100755
--- a/hooks/hooks.py
+++ b/hooks/hooks.py
@@ -143,11 +143,9 @@ was_i_primary = False
 
 
 def is_percona_source():
-    """This function will check the source repository of mongodb package
-    and return true if it comes from percona repo because it will change the whole charm
-    process like the config file structure.
-    An example of the percona mongodb repository is:
-    http://repo.percona.com/psmdb-44/apt  main
+    """Return true if MongoDB package comes from Percona repo.
+
+    It change the charm process if we are managing Percona MongoDB.
     """
     if "repo.percona.com" in config("source") or config("percona-mode"):
         return True
@@ -719,8 +717,6 @@ def leave_replset(master_node=None, host=None):
 
 
 def enable_replset(replicaset_name=None):
-    if is_percona_source():
-        default_mongodb_config = percona_mongodb_config
     retval = False
     if replicaset_name is None:
         juju_log("enable_replset: replicaset_name is None, exiting", level=DEBUG)
@@ -744,7 +740,10 @@ def enable_replset(replicaset_name=None):
                 mongodb_config = mongodb_conf(config_data)
             else:
                 mongodb_config = mongodb_conf_yaml(config_data)
-            retval = update_file(default_mongodb_config, mongodb_config)
+            if is_percona_source():
+                retval = update_file(percona_mongodb_config, mongodb_config)
+            else:
+                retval = update_file(default_mongodb_config, mongodb_config)
 
         juju_log("enable_replset will return: %s" % str(retval), level=DEBUG)
 
@@ -987,10 +986,6 @@ def enable_mongos(
     wait_for=default_wait_for,
     max_tries=default_max_tries,
 ):
-    if is_percona_source():
-        mongodb_pid_file = percona_mongodb_pid_file
-    else:
-        mongodb_pid_file = default_mongodb_pid_file
     juju_log("enable_mongos")
     if config_data is None or config_servers is None:
         juju_log("enable_mongos: config_data and config_servers are mandatory")
@@ -1142,11 +1137,12 @@ def config_changed():  # noqa: C901
     config_data = config()
     juju_log("config_data: {}".format(config_data), level=DEBUG)
     if is_percona_source():
-        default_mongodb_config = "/etc/mongod.conf"
-        with open(default_mongodb_config) as file:
+        mongodb_config_path = percona_mongodb_config
+        with open(mongodb_config_path) as file:
             mongodb_config = yaml.safe_load(file)
     else:
-        mongodb_config = open(default_mongodb_config).read()
+        mongodb_config_path = default_mongodb_config
+        mongodb_config = open(mongodb_config_path).read()
 
     if (
         config_data.changed("volume-ephemeral-storage")
@@ -1155,7 +1151,7 @@ def config_changed():  # noqa: C901
     ):
         config_changed_volume()
 
-    mongodb_config_hash = file_hash(default_mongodb_config)
+    mongodb_config_hash = file_hash(mongodb_config_path)
     # current ports
     if is_percona_source():
         current_mongodb_port = mongodb_config["net"]["port"]
@@ -1178,7 +1174,7 @@ def config_changed():  # noqa: C901
         mongodb_config = mongodb_conf(config_data)
     else:
         mongodb_config = mongodb_conf_yaml(config_data)
-    update_file(default_mongodb_config, mongodb_config)
+    update_file(mongodb_config_path, mongodb_config)
 
     if config_data["backups_enabled"]:
         backup_cronjob()
@@ -1203,7 +1199,7 @@ def config_changed():  # noqa: C901
     write_logrotate_config(config_data)
 
     # restart mongodb if the configuration file has changed.
-    if file_hash(default_mongodb_config) != mongodb_config_hash:
+    if file_hash(mongodb_config_path) != mongodb_config_hash:
         status_set("maintenance", "Restarting mongod")
         restart_mongod()
 
diff --git a/metadata.yaml b/metadata.yaml
index 4604c93..a4218ea 100644
--- a/metadata.yaml
+++ b/metadata.yaml
@@ -20,7 +20,6 @@ tags:
 series:
     - focal
     - bionic
-    - xenial
 provides:
   nrpe-external-master:
     interface: nrpe-external-master
diff --git a/tests/functional/requirements.txt b/tests/functional/requirements.txt
index 65ba4d7..97fc106 100644
--- a/tests/functional/requirements.txt
+++ b/tests/functional/requirements.txt
@@ -1,3 +1,3 @@
 git+https://github.com/openstack-charmers/zaza.git#egg=zaza
-pymongo
+pymongo==3.12.1
 python-openstackclient
diff --git a/tests/functional/tests/bundles/xenial.yaml b/tests/functional/tests/bundles/xenial.yaml
deleted file mode 100644
index a17f8b7..0000000
--- a/tests/functional/tests/bundles/xenial.yaml
+++ /dev/null
@@ -1,8 +0,0 @@
-series: xenial
-description: "mongodb-charm test bundle"
-applications:
-  mongodb:
-    num_units: 3
-    options:
-      replicaset: testset
-      backup_directory: /var/backups
diff --git a/tests/functional/tests/tests.yaml b/tests/functional/tests/tests.yaml
index 72347b4..cb8611c 100644
--- a/tests/functional/tests/tests.yaml
+++ b/tests/functional/tests/tests.yaml
@@ -1,9 +1,5 @@
 charm_name: mongodb
 tests:
-  - model_alias_xenial:
-      - tests.tests_mongodb.BasicMongodbCharmTest
-      - tests.tests_mongodb.ReplicatedMongodbCharmTest
-      - tests.tests_mongodb.XenialMongodbCharmTest
   - model_alias_bionic:
       - tests.tests_mongodb.BasicMongodbCharmTest
       - tests.tests_mongodb.CharmOperationTest
@@ -22,7 +18,6 @@ tests:
       - tests.tests_mongodb.ShardedMongodbCharmTest
       - tests.tests_mongodb.CharmOperationTest
 gate_bundles:
-  - model_alias_xenial: xenial
   - model_alias_bionic: bionic
   - model_alias_bionic_replicated: bionic-replicated
   - model_alias_focal: focal