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

[AAP-13408] Add support for custom actions #532

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benthomasson
Copy link
Contributor

Add action loading from a directory

@@ -435,6 +447,46 @@ async def _call_action(
except BaseException as e:
logger.error(e)
raise
elif action_plugin := self.find_action(action):
Copy link
Contributor

Choose a reason for hiding this comment

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

@benthomasson are we going to have special names for the custom actions so they don't conflict with our builtin action names. If the name conflict exists we will only run the builtin and not run the custom actions.

Copy link
Contributor

Choose a reason for hiding this comment

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

@benthomasson Since the code is the same for the 2 find_actions call can we move it into a separate function

         def run_action_plugin(self,....):
                action_plugin = self.find_action(action) or find_action(*split_collection_name(action))
                if not action_plugin:
                   return
                    
          try:
                await action_plugin["main"](
                    event_log=self.event_log,
                    inventory=inventory,
                    hosts=hosts,
                    variables=variables,
                    project_data_file=self.project_data_file,
                    source_ruleset_name=ruleset,
                    source_ruleset_uuid=ruleset_uuid,
                    source_rule_name=rule,
                    source_rule_uuid=rule_uuid,
                    rule_run_at=rule_run_at,
                    **action_args,
                )
            except Exception as e:
                logger.error("Error calling action %s, err %s", action, str(e))
            except BaseException as e:
                logger.error(e)
                raise

Copy link
Contributor

Choose a reason for hiding this comment

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

@benthomasson doing this search with every action call might not make sense, we could load all the actions in when we read the ruleset. Which also allows us to ensure that all the actions are valid before we start accepting the events and running the rule engine. if the actions are missing we can do an early termination to indicate that actions are missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@benthomasson are we going to have special names for the custom actions so they don't conflict with our builtin action names. If the name conflict exists we will only run the builtin and not run the custom actions.

We should support FQCN for actions. Names without collections prefixes are either builtin or loaded from a local directory. The precedence should be builtin and then local directory for non-FQCN.

@@ -134,6 +137,24 @@ def load_rulebook(collection, rulebook):
return yaml.safe_load(f.read())


def has_action(collection, action):
Copy link
Contributor

Choose a reason for hiding this comment

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

@benthomasson if we can take in an additional parameter additional_dirs, here which is the directory names we can search for the files in the collection location and an additional directory

    def has_action(collection, action, additional_dirs = []):
       dirs_to_search = additional_dirs
       dirs_to_search.append(EDA_ACTION_PATHS)
       return has_object(
        collection,
        action,
        dirs_to_search,
        ".py",
    )
           

source_rule_uuid=rule_uuid,
rule_run_at=rule_run_at,
**action_args,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

@benthomasson do we have to send the result of the action_plugin to the AAP server, after it has finished execution. Will there be a post_event/set_fact at the end of this action. Would it make sense to extract out some of the logic from builtin which does the reporting for us.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We probably should send the result. Moving the reporting code out of builtin into a different module is a good idea.

@Alex-Izquierdo
Copy link
Contributor

What is it the story/ticket related?

@benthomasson benthomasson changed the title Add support for custom actions [AAP-13408] Add support for custom actions Jun 21, 2023
@benthomasson
Copy link
Contributor Author

AAP-13408

@benthomasson
Copy link
Contributor Author

benthomasson commented Jun 21, 2023

Note to self: this needs a way to dynamically extend the rulebook schema or allow for arbitrary actions in the schema.

@Alex-Izquierdo
Copy link
Contributor

We could establish that each action has to be hosted in a folder and that it should contain action_name.py and action_name_schema.json files

@benthomasson benthomasson force-pushed the hello-ansible branch 4 times, most recently from 3456e39 to 1d55262 Compare January 18, 2024 17:11
@@ -92,6 +92,8 @@ async def run(parsed_args: argparse.Namespace) -> None:
startup_args.controller_url = parsed_args.controller_url
startup_args.controller_token = parsed_args.controller_token
startup_args.controller_ssl_verify = parsed_args.controller_ssl_verify
startup_args.source_dir = parsed_args.source_dir
Copy link
Contributor

Choose a reason for hiding this comment

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

@benthomasson Can these go into the settings and it can be accessed from anywhere. We have been moving most of the passed in args into the settings.

inventory,
hosts,
)
await action_plugin.main(metadata, control, **action_args)
Copy link
Contributor

Choose a reason for hiding this comment

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

@benthomasson Should we pass the metadata, control which are data classes as dictionaries into the action plugin so they can be tested outside of ansible rulebook otherwise they would have to know about the Metadata,Control data class definitions and if we change it it will effect the plugin. Also for the feedback should we be passing in a function that can they call to send the feedback to the eda-server. We should have a very loose coupling between action plugin and ansible-rulebook so each can be tested separately.

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.

3 participants