Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --all-tagged option; add and fix vctl tests #2882

Merged
merged 7 commits into from
Mar 8, 2022

Conversation

bonicim
Copy link
Collaborator

@bonicim bonicim commented Feb 11, 2022

Description

What is the change?

  • Adds --all-tagged option
  • Refactors start_agent, stop_agent
  • Changes 'nargs' input for start, stop, restart commands
  • Manually handle input validation for start, stop, and restart
  • Adds tests for --all-tagged, --tag, and starting an agent by uuid
  • Adds try/except to connection tests to check if the error message is an empty string;

Why is it needed?

  • Gives users the ability to start, stop, restart only tagged agents
  • Make start_agent and stop_agent more DRY
  • The current contract of vctl start, vctl stop, and vctl restart requires a positional argument; however, the --all-tagged does not require any input. Thus to maintain the current contract of the commands while supporting the --all-tagged option that doesn't require an input, the 'nargs' argument needs to change from '+' to '*'. This change will no longer use the automatic input validation that '+' offers (see Python docs). Thus, we need to manually handle input validation.
  • Ensures that control tests pass in CI pipeline; currently always failing, see https://github.com/VOLTTRON/volttron/actions/workflows/pytest-vctl.yml

Fixes #2881

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

See screen recording: https://youtu.be/nAYG6L7mKVU

Also, ran the control tests:

pytest volttrontesting/platform/control_tests/test_vctl_commands.py 

# Results
=== test session starts ==
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.11.0, pluggy-1.0.0 -- /home/bono/Workplace/volttron-project/volttron/env/bin/python
cachedir: .pytest_cache
rootdir: /home/bono/Workplace/volttron-project/volttron, configfile: pytest.ini
plugins: timeout-2.1.0, rerunfailures-10.2, asyncio-0.18.1
asyncio: mode=auto
collected 26 items                                                                                                                                                                                                                

volttrontesting/platform/control_tests/test_vctl_commands.py::test_needs_connection PASSED                                                                                                                                  [  3%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_needs_connection_with_connection[volttron_instance0] PASSED                                                                                              [  7%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_no_connection[volttron_instance0] PASSED                                                                                                                 [ 11%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_same_identity[volttron_instance0] PASSED                                                                                                         [ 15%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_with_wheel[volttron_instance0] PASSED                                                                                                            [ 19%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_with_wheel_bad_path[volttron_instance0] PASSED                                                                                                   [ 23%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args0] PASSED                                                                                                 [ 26%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args1] PASSED                                                                                                 [ 30%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args2] PASSED                                                                                                 [ 34%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args3] PASSED                                                                                                 [ 38%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args4] PASSED                                                                                                 [ 42%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args5] PASSED                                                                                                 [ 46%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args6] PASSED                                                                                                 [ 50%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-False-args7] PASSED                                                                                                [ 53%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-False-args8] PASSED                                                                                                [ 57%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-False-args9] PASSED                                                                                                [ 61%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-False-args10] PASSED                                                                                               [ 65%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args11] PASSED                                                                                                [ 69%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_install_arg_matrix[volttron_instance0-True-args12] PASSED                                                                                                [ 73%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_agent_filters[volttron_instance0] PASSED                                                                                                                 [ 76%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_vctl_start_stop_restart_by_uuid_should_succeed[volttron_instance0] PASSED                                                                                [ 80%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_vctl_start_stop_restart_by_tag_should_succeed[volttron_instance0] PASSED                                                                                 [ 84%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_vctl_start_stop_restart_by_all_tagged_should_succeed[volttron_instance0] PASSED                                                                          [ 88%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_vctl_start_stop_restart_should_raise_error_on_invalid_options[volttron_instance0-start] PASSED                                                           [ 92%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_vctl_start_stop_restart_should_raise_error_on_invalid_options[volttron_instance0-stop] PASSED                                                            [ 96%]
volttrontesting/platform/control_tests/test_vctl_commands.py::test_vctl_start_stop_restart_should_raise_error_on_invalid_options[volttron_instance0-restart] PASSED                                                         [100%]


== 26 passed, 2 warnings in 188.26s (0:03:08) ==

Associated Workflow also passes:

https://github.com/VOLTTRON/volttron/actions/runs/1895768008

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@bonicim bonicim requested a review from craig8 February 11, 2022 00:54
@bonicim bonicim changed the title Add log statements Add vctl commands to start, stop, restart tagged agents Feb 11, 2022
@craig8
Copy link
Contributor

craig8 commented Feb 11, 2022

@bonicim what issue is this against
please add it so we can re-look at the design of this parameter.

@bonicim
Copy link
Collaborator Author

bonicim commented Feb 14, 2022

@bonicim what issue is this against please add it so we can re-look at the design of this parameter.

This issue is logged in the internal JIRA board. I created issue #2881 on Github to track this publicly. I will update #2881 with a link to the Jira ticket.

@bonicim bonicim marked this pull request as draft February 16, 2022 23:35
@bonicim
Copy link
Collaborator Author

bonicim commented Feb 17, 2022

Updated PR based on feedback from internal meeting.

@bonicim bonicim marked this pull request as ready for review February 17, 2022 20:02
@bonicim bonicim marked this pull request as draft February 23, 2022 21:32
@bonicim bonicim changed the title Add vctl commands to start, stop, restart tagged agents Add --all-tagged option; add and fix vctl tests Feb 24, 2022
@bonicim
Copy link
Collaborator Author

bonicim commented Feb 24, 2022

Added tests for the following vctl commands:

  • vctl start (some_uuid)

  • vctl stop (some_uuid)

  • vctl restart (some_uuid)

  • vctl start --tag (some_tag)

  • vctl stop --tag (some_tag)

  • vctl restart --tag (some_tag)

  • vctl start --all-tagged

  • vctl stop --all-tagged

  • vctl restart --all-tagged

Also updated the assertion workflow for test_needs_connection_with_connection, test_no_connection, and test_needs_connection so that they don't fail in CI Workflows. See failed workflows in https://github.com/VOLTTRON/volttron/actions/workflows/pytest-vctl.yml.

Successful workflow at https://github.com/VOLTTRON/volttron/actions/runs/1895768008

@bonicim bonicim marked this pull request as ready for review February 24, 2022 23:03
@craig8
Copy link
Contributor

craig8 commented Mar 2, 2022

vctl start --all does not produce an error.

I would expect it to do the same thing as

vctl start foo when foo does not exist

start: error: agent not found: foo

This may be an issue for a different item however.

@craig8
Copy link
Contributor

craig8 commented Mar 2, 2022

Before this pull request the following would happen when passing --all

(volttron) gridappsd@gridappsd_dev_2004:/repos/volttron$ vctl start --all
usage: vctl command [OPTIONS] ... start [-h] [--name] [--tag] [--uuid] [-c FILE] [--debug] [-t SECS]
                                        [--msgdebug MSGDEBUG] [--vip-address ZMQADDR]
                                        pattern [pattern ...]
vctl command [OPTIONS] ... start: error: the following arguments are required: pattern

Because we are changing this, I think that either --foo or --anything should still raise an error if it's not one of the predefined ones that we have when viewing vctl start --help

Copy link
Contributor

@craig8 craig8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comments about command line interface.

@bonicim
Copy link
Collaborator Author

bonicim commented Mar 2, 2022

Before this pull request the following would happen when passing --all

(volttron) gridappsd@gridappsd_dev_2004:/repos/volttron$ vctl start --all
usage: vctl command [OPTIONS] ... start [-h] [--name] [--tag] [--uuid] [-c FILE] [--debug] [-t SECS]
                                        [--msgdebug MSGDEBUG] [--vip-address ZMQADDR]
                                        pattern [pattern ...]
vctl command [OPTIONS] ... start: error: the following arguments are required: pattern

Because we are changing this, I think that either --foo or --anything should still raise an error if it's not one of the predefined ones that we have when viewing vctl start --help

Good catch. Makes sense. I'll update the code to reflect that behavior and also add a test for this as well.

@bonicim bonicim marked this pull request as draft March 7, 2022 21:07
@bonicim
Copy link
Collaborator Author

bonicim commented Mar 8, 2022

Commit 6e10 disables "allow_abbrev" on the argparser. By default, "allow_abbrev" is set to true, which allows for abbreviations on long options. For example, the long option "--all-tagged" was shortened to "--all", thus allowing the command "vctl start --all" to be a legal command. For details, see https://docs.python.org/3.9/library/argparse.html#prefix-matching

@bonicim bonicim marked this pull request as ready for review March 8, 2022 20:56
@bonicim bonicim requested a review from craig8 March 8, 2022 20:56
@craig8 craig8 merged commit c308263 into VOLTTRON:develop Mar 8, 2022
bonicim pushed a commit to bonicim/volttron that referenced this pull request Sep 14, 2022
Add --all-tagged option; add and fix vctl tests
@bonicim bonicim mentioned this pull request Sep 14, 2022
12 tasks
craig8 added a commit that referenced this pull request Oct 3, 2022
* Disable abbrev on long options; add test

* add setup file to SCPAgent

* Fix exception order

* Remove RELEASE_NOTES.md

* added quick-start tutorial to documentation, under developing-volttron

* added quick-start tutorial to documentation, under developing-volttron

* Update to non-auth and modular auth for volttron.
Added dataclasses for parameters.
Moved and restructured code to break out rmq and zmq, and provide standardized APIs where applicable.

* Added 'vui' group to the generation of the web-users.json file in web/admin-endpoints.

* add readme to example C Agent

* added more details to the readme

* change readme file type to markdown

* fix headings

* remove extra parenthesis

* tested CSV agent and driver, and added to README

* updated the readme and added execute permission to the launch_my_historian.sh script

* merge

* add readme

* added readme with link to docs

* fix location of test_certs file in workflow for platform tests

* Added ZMQ/RMQ Client Authorization for Auth subsystem.
Updated terminology in main/config.
Updated allow auth to be True by default.
Cleaned up some imports.

* hot fix sphinx ref syntax error

* fix for #2901 #2864 (fix for rmq test cases with pika v1.2.0)
Work around for #2938

* hot fix CLI command error, sphinx syntax error

* Enabling rmq fixture for testing

* bug fix based on tests

* hot fix CLI command error, sphinx syntax error

* Updates to fix import issues.

* Added agent/config, which includes both agent/configs and agent/configs/filename endpoints and their test to vui_endpoints.

* Added agent/config, which includes both agent/configs and agent/configs/filename endpoints and their test to vui_endpoints.

* Merged Platforms Agents Configs endpoints in vui_endpoints.py. Minor fixes and added documentation for these endpoints.

* Fix for issue #2950. Handle unexpected exception in process loop

* fix for issue #2945

* Removed initial non-auth tests.
Added handling for allow_auth and AUTH_ENABLED to manage use of auth in config file.
Minor code cleanup.

* Added is_web_enabled, and is_auth_enabled to utils __all__.
Added AuthFileIndexError to auth __init__.py
Updated agent core to use is_auth_enabled.
Fixed mocked auth service to work with new auth service design.

* debugging mysql unit tests

* debugging mysql unit tests

* debugging mysql unit tests

* debugging mysql unit tests

* debugging mysql unit tests

* debugging mysql unit tests

* debugging mysql unit tests

* raised an error when config path doesnt exist

* Fixed import errors for tests.
Fixed connection issue to remote platforms.
Fixed rpc call names in web and web tests.

* created tests for load_config

* add readme for scheduler example agent

* add readme and small updates to agent.py file

* remove comment from python file and added it into a readme

* renamed config file and added in installation instructions to readme

* Fixed web to use appropriate auth methods.
Modified ZMQServerAuthentication and ZMQAuthorization to reference AuthService instance directly.

* updated readme to be more verbose and updated some comments in settings.py

* create readme using comment from python file, updated comments in settings.py

* Update README.md

* update readme

* updated readme based on PR comments

* updated readme

* add readme following similar structure to other stand alone example agents

* update readme

* update readme

* update numbering in readme

* created README and removed comments from config

* updated readme, and reverted changes to config

* change back to simpleweb

* get rid of the repetivie driver_type: fakedriver

* Update fake.config

remove comma to make json compliant

* add setup file, add readme, add certs verification to do_rpc function

* Added documentation for non-auth implementation, and modular approach to auth authentication and authorization.

* Updated pytests configuration to ignore contrib and unsupported files.
Updated openadr ven agent to ignore tests if dependencies aren't found.
Update actuator agent to ignore tests if local dependencies aren't found.
Increased default timeout to 300.

* delete WebRPC example agent, functionality will be moved over to jsonrpc endpoint of SimpleWebAgent

* Fixed missing external_platform argument in vui_endpoints.handle_platforms_agents_running._agent_running()

* Fixed accidental string conversion in vui_endpoints.handle_platforms_agents_enabled().

* Added checks for specific dependencies for MQTTHistorian.
Increased timeout for vc and platform driver install.
Added auth_enabled to PlatformWrapper.
Added Non-auth cases to volttron_instance and volttron_web_instance.

* Cleanup stop-volttron

* Update docs link

* small fix to BaseClientAuthorization's connect_remote_platform method.

* Minor ZMQAuthorization updates.

* Monkey patching in Control to fix gevent monkeypatching warning.

* Update requirements.py

Bump gevent to latest.

* Changed "route_options" to "links". Fixed behaviour of agents/vip_identity endpoint. Fixed tests.

* Delete IntervalValues.csv

Removed accidental addition to commit.

* Refactored connect_remote_platform into core.
Handled auth case in connect_remote_platform for zmq case.
 Cleaned up monkeypatch implementation for main, control_parser, and vip.

* Fixed rmq auth connection issues

* fixing merge conflict

* Pinned value for idna to fix issue with VC login authentication.

* Updated VC test to fail appropriately.
Modified is_running_in_container to handle subprocess error in call.

* Minor fixes to remote connection.

* WARNING: Test improvement to RMQ dependencies script. May remove before rc.

* minor fixes to rmq core platform connection

* Fixed RMQ connections.
Fixed web authentication page for RMQ.
Fixed CSR handling.

* Undid debug log messages

* Modified context to use TLSv1.

* Removed deprecated connect_remote_platform from auth subsystem.
Added grequests.
Updated connection_params for build_remote_connection_params.
Fixed url_address missing from RMQConnectionAPI.

* Remove duplicated code.

* Calling base class constructor

* fixed auth tests failing in non auth. Auth tests shouldn't run in non auth version

* Fixed typo

* Fix vc test using correct grequest api

* Fixes for forward historian connection between rmq and zmq instances

* Added toctree link to web-api documentation.

* Added toctree entries for web-api documentation.

* Removed upgrade option from bootstrap.py and made this the default.

* Remove updgrade option from bootstrap.py and make this behavior the default.

* federation/shovel fixes

* Use volttron-build-action v4 for rmq actions

* updated to using v4 of volttron-build-action

* Removed downstream tests for now.

* update version in the correct spot.

* Tests without connections shouldn't take volttron_instance.

* Update version/license and pinned requirements

* fix for issue #2945

* non auth fixes primarily for rmq

* moved code to deprecated

* moved code to deprecated

* reverting accidental commit

* reverting accidental commit

* Update driver tests; cleanup logs for security agent

* Fix for issue #2992

* fix for non auth

* commented out deprecated dbs for historian testing

* deleted test module that test only with the default config. agents that deal with database always need db details so agent won't work with default/empty configs

* Added fixture to skip for non auth case

* Added support for getting forecast data using station id. Issue number #2994

* removed crate and mongo historian from test param as we do not support those historians any more

* Added link to restful API from agent web framework. Renamed link from VUI to "Restful Web Interface".

* removed unused import - #2997

* Protect import statements when rabbitmq not installed

* Added link to restful API from agent web framework. Renamed link from VUI to "Restful Web Interface".

* Fix for issue #2999

* Fix for issue #2997

* Update volttron_platform_fixtures.py

Uncommented that was commented for debugging

* Increased sleep time in test_file_watcher to avoid timing issue with rmq.

* Add auth check

* Fix import statement

* skip rmq specific tests if pika is not available

* removed unused import

* test fixes

* Merge pull request #2882 from bonicim/feature/vctl-tag

Add --all-tagged option; add and fix vctl tests

* xfailed web tests as they are redundent.

* Cleanup commented out code

* Cleanup auth.py code add MagicMock to gitignore.

* Reformat code.

* Fix web test auth enabled is required for passing test.

* Fix rmq tests that weren't being run for web.

* RMQ web auth tests aren't valid.
Because the local agents are added on demand, so they need to be reworked.

* Fixed installation issue timeout with small buffer.

* param for fixture is passed through platform web instance.
xfail for rmq<-zmq web and zmq<-rmq web.

* param for fixture is passed through platform web instance.
xfail for rmq<-zmq web and zmq<-rmq web.

* Update README.md

Pinned helics pypi version to 2.8.0. 
Fixes Issue #3012

* skip rmq tests.

* 20 second wait is not required.

* Disabled webcsr tests for now.

* Fix spelling error.

* Fix spelling error.

* Refactor PlatformDriver test (#3013)

* Refactor PlatformDriver tests

* Reformat test to PEP 8 compliance

* Tests/fix test global settings (#3014)

* Refactor config store test fixture

* Reformat code to be PEP 8 compliant

* Tests/fix test device groups (#3016)

* Fix test_device_groups

* Reformat to PEP8 compliance

* Add logic to parse unsupported objects (#3021)

* 8.2 doc updates for non auth and left tree reorg (#3019)

* Updated Read the docs for non auth mode. moved docs around to reduce the length of left side nav tree

* Updated Read the docs for non auth mode. moved docs around to reduce the length of left side nav tree

* Update mongodb-related dependencies (#3005)

* hot-fix-doc/#3030There-are-two-menu-items-with-quickstart-to-two-different-locations (#3032)

* Remove unused import (#3017)

* Fix VOLTTRON_HOME substitution

* Remove unused import

Co-authored-by: Chandrika <schandrika@users.noreply.github.com>
Co-authored-by: Craig <3979063+craig8@users.noreply.github.com>

* Modify expected output (#3004)

* Modify expected output

* Update test

* Remove unused imports (#3018)

* Fix VOLTTRON_HOME substitution

* Remove unused imports

Co-authored-by: Chandrika <schandrika@users.noreply.github.com>
Co-authored-by: Craig <3979063+craig8@users.noreply.github.com>

* small updates to README (#2962)

* Update openadr docs (#3034)

* Renamed secure mode to agent isolation mode - fix for #3035 (#3036)

* fix for issue #3035

* renamed script used to stop agent running in isolation mode

* Update main copyright for rtd and fix formatting (#3033)

* Update vc list docs
Fix #3029

* Modify Copyright Date
Fix #3023

* Create CODE_OF_CONDUCT.md (#3040)

* Update issue templates (#3039)

* Update issue templates

* Update bug_report.md

Remove smartphone from bug template.
Add Volttron Version

* Minor fix for volttron-upgrade command (#3038)

* fix for issue #3035

* renamed script used to stop agent running in isolation mode

* Fix for issue #3037

* using pathlib.Path instead of os path

* #Updated 8.x upgrade doc about agent-isolation-mode - #3025 (#3042)

* #Updated 8.x upgrade document to include details about agent-isolation-mode. fixes #3025

* #Updated 8.x upgrade document to include details about agent-isolation-mode. fixes #3025

* Updated release history. fixes #3031 (#3043)

* More rtd updates. updated broken links, cleaned up left side tree (#3044)

Co-authored-by: Mark Bonicillo <mark.bonicillo@pnnl.gov>
Co-authored-by: gwenkidd <gwendolyn.kidd@pnnl.gov>
Co-authored-by: Cody Scott <cody.j.b.scott@gmail.com>
Co-authored-by: Kefei Mo <kefeimo@gmail.com>
Co-authored-by: gilb842 <spencer.gilbride@pnnl.gov>
Co-authored-by: David M. Raker <david.raker@utoledo.edu>
Co-authored-by: schandrika <chandrika@pnnl.gov>
Co-authored-by: Gokarna Jung Bhandari <gokarna.jung.bhandari@gmail.com>
Co-authored-by: Chandrika <schandrika@users.noreply.github.com>
Co-authored-by: Shwetha Niddodi <shwetha.niddodi@pnnl.gov>
Co-authored-by: sgilbride <49735262+sgilbride@users.noreply.github.com>
Co-authored-by: David M. Raker <david.raker@pnnl.gov>
Co-authored-by: Gwen Kidd <95827084+gwenkidd@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants