[minigraph] new workflow for golden path (#2396)

#### What I did
Change the behavior that load_minigraph will consume golden config by default.
New behavior:
`config load_minigraph`: No longer consume golden config.
`config load_minigraph --golden_config`: Consume default golden config. /etc/sonic/golden_config_db.json
`config load_minigraph --golden_config FilePath`: Consume golden config with FilePath
#### How I did it
Make golden_config click.Option() and add an argument for golden config path.
#### How to verify it
UT test.
#### Previous command output (if the output of a command-line utility has changed)
sudo config load_minigraph -h
Usage: config load_minigraph [OPTIONS]

Reconfigure based on minigraph.

Options:
-y, --yes
-n, --no_service_restart Do not restart docker services
-t, --traffic_shift_away Keep device in maintenance with TSA
-p, --golden_config_path TEXT specify Golden Config path
-?, -h, --help Show this message and exit.
#### New command output (if the output of a command-line utility has changed)
admin@vlab-01:~$ sudo config load_minigraph --golden_config_path -h
Usage: config load_minigraph [OPTIONS]

  Reconfigure based on minigraph.

Options:
  -y, --yes
  -n, --no_service_restart       Do not restart docker services
  -t, --traffic_shift_away       Keep device in maintenance with TSA
  -o, --override_config          Enable config override. Proceed with default path.
  -p, --golden_config_path TEXT  Provide golden config path to override. Use with --override_config
  -h, -?, --help                 Show this message and exit.

Author: wen587

config/main.py

@@ -1727,9 +1727,10 @@ def load_mgmt_config(filename):
                 expose_value=False, prompt='Reload config from minigraph?')
 @click.option('-n', '--no_service_restart', default=False, is_flag=True, help='Do not restart docker services')
 @click.option('-t', '--traffic_shift_away', default=False, is_flag=True, help='Keep device in maintenance with TSA')
-@click.option('-p', '--golden_config_path', help='The path of golden config file')
+@click.option('-o', '--override_config', default=False, is_flag=True, help='Enable config override. Proceed with default path.')
+@click.option('-p', '--golden_config_path', help='Provide golden config path to override. Use with --override_config')
 @clicommon.pass_db
-def load_minigraph(db, no_service_restart, traffic_shift_away, golden_config_path):
+def load_minigraph(db, no_service_restart, traffic_shift_away, override_config, golden_config_path):
     """Reconfigure based on minigraph."""
     log.log_info("'load_minigraph' executing...")
 
@@ -1802,20 +1803,19 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, golden_config_pat
     # Keep device isolated with TSA 
     if traffic_shift_away:
         clicommon.run_command("TSA", display_cmd=True)
-        if golden_config_path or not golden_config_path and os.path.isfile(DEFAULT_GOLDEN_CONFIG_DB_FILE):
+        if override_config:
             log.log_warning("Golden configuration may override System Maintenance state. Please execute TSC to check the current System mode")
             click.secho("[WARNING] Golden configuration may override Traffic-shift-away state. Please execute TSC to check the current System mode")
 
     # Load golden_config_db.json
-    if golden_config_path:
+    if override_config:
+        if golden_config_path is None:
+            golden_config_path = DEFAULT_GOLDEN_CONFIG_DB_FILE
         if not os.path.isfile(golden_config_path):
             click.secho("Cannot find '{}'!".format(golden_config_path),
                         fg='magenta')
             raise click.Abort()
         override_config_by(golden_config_path)
-    else:
-        if os.path.isfile(DEFAULT_GOLDEN_CONFIG_DB_FILE):
-            override_config_by(DEFAULT_GOLDEN_CONFIG_DB_FILE)
 
     # We first run "systemctl reset-failed" to remove the "failed"
     # status from all services before we attempt to restart them

tests/config_test.py

@@ -411,52 +411,39 @@ def is_file_side_effect(filename):
                 assert result.exit_code == 0
                 assert expected_output in result.output
 
-    def test_load_minigraph_with_golden_config(self, get_cmd_module, setup_single_broadcom_asic):
-        with mock.patch(
-            "utilities_common.cli.run_command",
-            mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command:
-            (config, show) = get_cmd_module
-            db = Db()
-            golden_config = {}
-            self.check_golden_config(db, config, golden_config,
-                                     "config override-config-table /etc/sonic/golden_config_db.json")
-
-    def check_golden_config(self, db, config, golden_config, expected_output):
-        def is_file_side_effect(filename):
-            return True if 'golden_config' in filename else False
-        with mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)):
-            runner = CliRunner()
-            result = runner.invoke(config.config.commands["load_minigraph"], ["-y"], obj=db)
-            print(result.exit_code)
-            print(result.output)
-            assert result.exit_code == 0
-            assert expected_output in result.output
-
     def test_load_minigraph_with_non_exist_golden_config_path(self, get_cmd_module):
         def is_file_side_effect(filename):
             return True if 'golden_config' in filename else False
         with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command, \
                 mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)):
             (config, show) = get_cmd_module
             runner = CliRunner()
-            result = runner.invoke(config.config.commands["load_minigraph"], ["-p", "non_exist.json", "-y"])
+            result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "--golden_config_path", "non_exist.json", "-y"])
             assert result.exit_code != 0
             assert "Cannot find 'non_exist.json'" in result.output
 
-    def test_load_minigraph_with_golden_config_path(self, get_cmd_module):
+    def test_load_minigraph_with_specified_golden_config_path(self, get_cmd_module):
         def is_file_side_effect(filename):
             return True if 'golden_config' in filename else False
         with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command, \
                 mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)):
             (config, show) = get_cmd_module
             runner = CliRunner()
-            result = runner.invoke(config.config.commands["load_minigraph"], ["-p", "golden_config.json", "-y"])
-            print(result.exit_code)
-            print(result.output)
-            traceback.print_tb(result.exc_info[2])
+            result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "--golden_config_path",  "golden_config.json", "-y"])
             assert result.exit_code == 0
             assert "config override-config-table golden_config.json" in result.output
 
+    def test_load_minigraph_with_default_golden_config_path(self, get_cmd_module):
+        def is_file_side_effect(filename):
+            return True if 'golden_config' in filename else False
+        with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command, \
+                mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)):
+            (config, show) = get_cmd_module
+            runner = CliRunner()
+            result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "-y"])
+            assert result.exit_code == 0
+            assert "config override-config-table /etc/sonic/golden_config_db.json" in result.output
+
     def test_load_minigraph_with_traffic_shift_away(self, get_cmd_module):
         with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command:
             (config, show) = get_cmd_module
@@ -477,7 +464,7 @@ def is_file_side_effect(filename):
                 db = Db()
                 golden_config = {}
                 runner = CliRunner()
-                result = runner.invoke(config.config.commands["load_minigraph"], ["-ty"])
+                result = runner.invoke(config.config.commands["load_minigraph"], ["-ty", "--override_config"])
                 print(result.exit_code)
                 print(result.output)
                 traceback.print_tb(result.exc_info[2])