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 support for external plugins #12

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

lpiwowar
Copy link
Collaborator

This patch ensures that the test operator takes advantage of the new parameters of the tempest container:
- TEMPEST_EXTERNAL_PLUGIN_GIT_URL
- TEMPEST_EXTERNAL_PLUGIN_CHANGE_URL
- TEMPEST_EXTERNAL_PLUGIN_REFSPEC

The user can use this parameters by specifying the following values for the Tempest CR:
- externalPluginGitURL
- externalPluginChangeURL
- externalPluginRefspec

@lpiwowar lpiwowar force-pushed the lpiwowar/feature/tempest-external-params branch 4 times, most recently from 39faea8 to ff2f65a Compare November 23, 2023 10:11
@lpiwowar lpiwowar changed the title [DNM] Add support for external plugins Add support for external plugins Nov 23, 2023
@lpiwowar lpiwowar force-pushed the lpiwowar/feature/tempest-external-params branch 2 times, most recently from 2bd86a4 to 5ad6f93 Compare November 23, 2023 13:36
@@ -23,6 +23,15 @@ spec:
# smoke: false
# serial: false
# parallel: true
# externalPluginURL:
Copy link
Collaborator

Choose a reason for hiding this comment

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

so the first item in externalPluginRefspec is used together with the first one in externalPluginChangeURL and applied on first item from externalPluginURL?

What if I want to download barbican-tempest-plugin from master and a specific change from neutron-tempest-plugin? Do I need to write something like this?

externalPluginURL:
  - "https://opendev.org/openstack/barbican-tempest-plugin.git"
  - "https://opendev.org/openstack/neutron-tempest-plugin.git"
externalPluginChangeURL:
  - ""
  - "https://review.opendev.org/openstack/neutron-tempest-plugin"
externalPluginRefspec:
  - ""
  - "refs/changes/97/896397/2"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So the first item in externalPluginRefspec is used together with the first one in externalPluginChangeURL and applied on the first item from externalPluginURL?

  • Yes, exactly.

What if I want to download barbican-tempest-plugin from master and a specific change from neutron-tempest-plugin?

  • If you want to apply changes only to a subset of repositories specified in externalPluginGitURL then you can do it like this:
externalPluginURL:
  - "https://opendev.org/openstack/neutron-tempest-plugin.git"
  - "https://opendev.org/openstack/barbican-tempest-plugin.git" # <- Put the repository without change at the end
externalPluginChangeURL:
  - "https://review.opendev.org/openstack/neutron-tempest-plugin"
externalPluginRefspec:
  - "refs/changes/97/896397/2"

OR like this:

externalPluginURL:
  - "https://opendev.org/openstack/neutron-tempest-plugin.git"
  - "https://opendev.org/openstack/barbican-tempest-plugin.git"
externalPluginChangeURL:
  - "-" # Use dash to indicate no change is needed
  - "https://review.opendev.org/openstack/neutron-tempest-plugin"
externalPluginRefspec:
  - "-" # Use dash to indicate no change is needed
  - "refs/changes/97/896397/2"

I updated the documentation so it is more clear. Maybe there is a better way to name the parameters (?) but nothing crosses my mind right now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I personally don't like the approach where we would have "-" - I'm almost 100% sure that users will not pass their plugins correctly :/

what about something like this?

externalPluginURL:
  - "https://opendev.org/openstack/barbican-tempest-plugin.git"
  - "https://opendev.org/openstack/neutron-tempest-plugin.git":
         - "https://review.opendev.org/openstack/neutron-tempest-plugin"
         - "refs/changes/97/896397/2"  

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, I can modify it to accept this format. Shouldn't be that difficult. I will update the patch.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kopecmartin, so it turns out to be difficult after all :D. When you define externalPluginURL like this then there is no nice way how to define the type of externalPluginURL as it can be:

  • a list of strings and
  • a list of dictionaries or
  • a combination of both.

It can be either a list of strings or a list of dictionaries. It would be probably possible to bypass this but I guess it would not be really openshifty. What do you think about this?

externalPlugin:
  - repository: "https://opendev.org/openstack/barbican-tempest-plugin.git"
  - repository: "https://opendev.org/openstack/neutron-tempest-plugin.git"
    changeRepository: "https://review.opendev.org/openstack/neutron-tempest-plugin"
    changeRefspec: "refs/changes/97/896397/2"

Copy link
Collaborator

Choose a reason for hiding this comment

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

this looks much better, i definitely like it more than the previous solution

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated the patch. I just want to make sure that it works therefore I added a WIP flag for now. I will remove it tomorrow morning once I will test it properly.

@lpiwowar lpiwowar force-pushed the lpiwowar/feature/tempest-external-params branch 3 times, most recently from 81859bc to 54b453a Compare November 30, 2023 17:30
@lpiwowar lpiwowar changed the title Add support for external plugins [WIP] Add support for external plugins Nov 30, 2023
@lpiwowar lpiwowar force-pushed the lpiwowar/feature/tempest-external-params branch from 54b453a to 9db704c Compare December 1, 2023 12:28
This patch ensures that the test operator takes advantage of the new
parameters of the tempest container:
    - TEMPEST_EXTERNAL_PLUGIN_GIT_URL
    - TEMPEST_EXTERNAL_PLUGIN_CHANGE_URL
    - TEMPEST_EXTERNAL_PLUGIN_REFSPEC

The user can use these parameters by specifying the following values
for the Tempest CR:
    - tempestRun.externalPlugin.repository
    - tempestRun.externalPlugin.changeRepository
    - tempestRun.externalPlugin.changeRefspec
@lpiwowar lpiwowar force-pushed the lpiwowar/feature/tempest-external-params branch from 9db704c to 4a98fb2 Compare December 1, 2023 12:29
@lpiwowar lpiwowar changed the title [WIP] Add support for external plugins Add support for external plugins Dec 1, 2023
@lpiwowar lpiwowar requested a review from kopecmartin December 1, 2023 16:32
@kopecmartin
Copy link
Collaborator

/lgtm

Copy link
Collaborator

@kopecmartin kopecmartin left a comment

Choose a reason for hiding this comment

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

lgtm

@kopecmartin kopecmartin merged commit 1e53401 into main Dec 20, 2023
2 checks passed
@lpiwowar lpiwowar deleted the lpiwowar/feature/tempest-external-params branch January 17, 2024 09:52
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