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

Allow plugin ordering #1684

Closed
mtulio opened this issue Apr 28, 2022 · 6 comments · Fixed by #1753
Closed

Allow plugin ordering #1684

mtulio opened this issue Apr 28, 2022 · 6 comments · Fixed by #1753
Labels
kind/enhancement New or improved functionality lifecycle/active Actively being worked on

Comments

@mtulio
Copy link

mtulio commented Apr 28, 2022

Describe the solution you'd like

References #631 #432 #39

Plugins should be able to run in a predetermined order.

Anything else you would like to add:

As well described on #631 , the plugins could be able to run in a certain order configured on their manifests. The default behavior could stay the current execution (all in parallel), but it would be nice to run, for example in serial mode or a mix: A->B->C->(D&E)->F.

Also could be interesting if there's one flag if one plugin has failed, the following ones will not be executed (maybe also marked as failed?). It can be useful in preflights plugins which usually run on the begging to make some prechecks (like minimal resources, minimal performance checks, etc).

@johnSchnake johnSchnake added the kind/enhancement New or improved functionality label Apr 28, 2022
@johnSchnake
Copy link
Contributor

Thanks for the submission and for the specific ideas related to use cases and it is a good point to raise about whether or not to continue with execution or to stop if one fails.

Like we talked about in slack, this idea always leads me to think about passing data along the chain as well but that has to be out of scope for now if we want to get this in.

@johnSchnake
Copy link
Contributor

Here was my PR for this in the past; I might take a look again. I believe it did work but it looks like I was partially concerned about implementing a feature like this without fully thinking about all the potential ramifications. We never got more push for it from the community so it just fizzled out after that (which is why I'm glad you reached out about it).

@mtulio
Copy link
Author

mtulio commented Apr 29, 2022

Like we talked about in slack, this idea always leads me to think about passing data along the chain as well but that has to be out of scope for now if we want to get this in.

Yes, that will be interesting to have some way to exchange information between plugins, maybe something like a pub/sub?!. As I commented on Slack, I was using the field message from the report progress endpoint to write the state, and reading it (.msg attribute from the plugin's progress structure) to make decisions and exchange a few information/states between them, but having an official channel will be nice! :)

{
  "plugins": [
    {
      "plugin": "kube-conformance",
      "node": "global",
      "status": "running",
      "result-status": "",
      "result-counts": null,
      "progress": {
        "name": "kube-conformance",
        "node": "global",
        "timestamp": "2022-04-29T13:22:17.288591969Z",
        "msg": "status=running",
        "total": 345,
        "completed": 39
      }
    },
    {
      "plugin": "cert-level1",
      "node": "global",
      "status": "running",
      "result-status": "",
      "result-counts": null,
      "progress": {
        "name": "cert-level1",
        "node": "global",
        "timestamp": "2022-04-29T13:22:10.889135367Z",
        "msg": "status=waiting-for=kube-conformance=(0/-314/0)=[0/100]",
        "total": 81,
        "completed": 0
      }
    },
    {
      "plugin": "cert-level2",
      "node": "global",
      "status": "running",
      "result-status": "",
      "result-counts": null,
      "progress": {
        "name": "cert-level2",
        "node": "global",
        "timestamp": "2022-04-29T13:22:13.853217982Z",
        "msg": "status=blocked-by=cert-level1=(0/-81/0)=[0/100]",
        "total": 17,
        "completed": 0
      }
    },
    {
      "plugin": "cert-level3",
      "node": "global",
      "status": "running",
      "result-status": "",
      "result-counts": null,
      "progress": {
        "name": "cert-level3",
        "node": "global",
        "timestamp": "2022-04-29T13:22:15.864117893Z",
        "msg": "status=blocked-by=cert-level2=(0/-17/0)=[0/100]",
        "total": 0,
        "completed": 0
      }
    }
  ],
  "status": "running",
  "tar-info": {
    "name": "",
    "created": "0001-01-01T00:00:00Z",
    "sha256": "",
    "size": 0
  }
}

@mtulio
Copy link
Author

mtulio commented Apr 29, 2022

Here was my PR for this in the past; I might take a look again. I believe it did work but it looks like I was partially concerned about implementing a feature like this without fully thinking about all the potential ramifications. We never got more push for it from the community so it just fizzled out after that (which is why I'm glad you reached out about it).

Could you please share the PR link?

which is why I'm glad you reached out about it

Cool! We are here to help! Sonobuoy is amazing, happy to contribute to its improvement.

@johnSchnake
Copy link
Contributor

Oop, didnt realize I forgot to paste the link: #627

@johnSchnake
Copy link
Contributor

@andrewyunt has picked this up and is tweaking my old PR to start. I think we'll have a PR soon to iterate on/discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New or improved functionality lifecycle/active Actively being worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants