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

Compatibility with Avocado #3295

Merged

Conversation

clebergnu
Copy link
Contributor

@clebergnu clebergnu commented Dec 8, 2021

Hi Avocado-VT developers and maintainers,

This is a PR that needs a bit of special attention from you. While it looks just like CI level updates, it proposes a decoupling between the development branch of Avocado and Avocado-VT. This should allow for Avocado to develop new features faster, while still considering compatibility between Avocado and Avocado-VT.

A couple of important points:

  1. Every new Avocado development sprint includes a week for stabilization and bug fixes. If changes are introduced in the latest Avocado development that causes a compatibility issue, there would still be one week to work on bringing back compatibility.

  2. The goal continues to be having all Avocado and Avocado-VT regular releases compatible, but that will depend on available resources. Compromise scenarios may be:
    2.1) Every "even" release is compatible (or every odd release, effectively every 2 releases)
    2.2) Only LTS versions are compatible

This is also an invitation for developers to work on this specific area and make the primary goal a reality. If we fail to have compatibility achieved for every release, we need to understand the resource limitations and settle on one of the other scenarios (every couple of
releases or LTS releases).

@luckyh @chunfuwen @pevogam (and everyone else) feedback is very welcome here!

Even Avocado 69.x LTS has reached EOL, so it makes sense to require at
least the older (but still active) LTS version.

With that, we don't have to guess the pre/post plugin available.

Signed-off-by: Cleber Rosa <[email protected]>
Avocado has the main development branch, but also branches for the LTS
versions that are currently active.

This adds an extra set of Cirrus CI tasks that check Avocado against
those branches and also against the LTS releases.

Signed-off-by: Cleber Rosa <[email protected]>
Avocado has taken some steps to allow faster development of new
features, while still caring for compatibility.

Out of those, the most relevant one is the existance of LTS releases,
which should can receive fixes and be used for a long time with a
predictable experience.

With the release of 92.X, and the the removal of the legacy "runner"
architecture in Avocado (among other changes), it's necessary to
detach Avocado-VT from the very latest Avocado commits.

By "allowing failures" for one specific job, we can monitor the
"compatibility delta" between Avocado and Avocado-VT, and work on
compatibility issues with more time and better planning.

The goal continues being having all Avocado and Avocado-VT regular
releases compatible, but that will depend on available resources.
One compromise scenario may be having at least every "even" release
compatible.  Worst case scenario, is having every new (and current LTS
version compatible).

This is also an invitation for developers to work on this specific are
and make the goal a reality, and if we fail to have compatibility
achieved for every release, we need to understand the resource
limitations and settle on one of the other scenarios (every couple of
releases or LTS releases).

Signed-off-by: Cleber Rosa <[email protected]>
@pevogam
Copy link
Contributor

pevogam commented Dec 9, 2021

Hi @clebergnu, thanks for the good summary on the general current stance about the coupling between the two projects, we definitely need refreshing like this to have a proper overview of what changed since the last time we talked about it.

On the Avocado I2N plugin side, we already had to reduce coupling because of the exact same reasons to just LTS releases. Previously, lots of changes were too frequent and too breaking for us and due to our limited resources we had to make a choice which was similar to the one you are suggesting. My greatest concern in this direction is that increased decoupling could lead to large and eventually exponentially (or at least nonlinearly) more costly adaptations to compensate for it. To counteract this, we have a CI that constantly pulls the current master versions of Avocado and Avocado VT and detects issues earlier rather than later. This helps us reduce the chance of compounding deviations and keep them manageable.

So I am fine even with LTS version tracking as long as:

  1. There are no tags on the Avocado VT side which might confuse and mislead users when there is no verified compatibility for the tag version.

  2. There is indeed a best effort (hopefully more automated and less human-reliant one) to keep compatibility and avoid compounding of changes.

@pevogam
Copy link
Contributor

pevogam commented Dec 14, 2021

Coming to think more about this, I think there are some new downsides to it I have to point out:

  1. Avocado must keep backwards compatibility and a stable API not just for Avocado VT but for downstream users and plugins in general. This means that Avocado VT could be a nice middle test ground for this and is not the only API change constraint Avocado has to deal with anyway.

  2. Some automated CI we have build and perhaps other people make sure to test the most recent master branch of both Avocado and Avocado VT and our ability to detect and report issues in a granular fashion will be negatively impacted by such lack of sync. While I agree fixes cannot be made available at a very high pace I would still expect that reporting them can happen the moment they are detected and not fixing them can have some upper limit because the window of unadapted fix is also a window in which we cannot run the CI to detect other outstanding issues.

Copy link
Member

@beraldoleal beraldoleal left a comment

Choose a reason for hiding this comment

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

LGTM.

@clebergnu
Copy link
Contributor Author

clebergnu commented Dec 16, 2021

Hi @clebergnu, thanks for the good summary on the general current stance about the coupling between the two projects, we definitely need refreshing like this to have a proper overview of what changed since the last time we talked about it.

On the Avocado I2N plugin side, we already had to reduce coupling because of the exact same reasons to just LTS releases. Previously, lots of changes were too frequent and too breaking for us and due to our limited resources we had to make a choice which was similar to the one you are suggesting. My greatest concern in this direction is that increased decoupling could lead to large and eventually exponentially (or at least nonlinearly) more costly adaptations to compensate for it. To counteract this, we have a CI that constantly pulls the current master versions of Avocado and Avocado VT and detects issues earlier rather than later. This helps us reduce the chance of compounding deviations and keep them manageable.

So I am fine even with LTS version tracking as long as:

  1. There are no tags on the Avocado VT side which might confuse and mislead users when there is no verified compatibility for the tag version.

I think this is a very important point. So far we've had CI guaranteeing (some) compatibility on every commit so it applies to every release.

Given the many options I presented originally, let me say it clearly that I agree your this point, and if it depends on my, the initial proposal is to use the one week we now have before every Avocado release (after feature freeze and before release) to work on bugs and compatibility issues. This would guarantee that every Avocado and Avocado-VT tagged versions continues to have compatibility.

  1. There is indeed a best effort (hopefully more automated and less human-reliant one) to keep compatibility and avoid compounding of changes.

Agreed. On the Avocado-VT side, there could be more tests tp-qemu or tp-libvirt tests added to the CI to further increase compatibility coverage. On the Avocado side, the "deployment checks" we used to do run manually are now available on GH Actions and can be expanded to deploy from the latest Avocado-VT source code.

@luckyh
Copy link
Contributor

luckyh commented Dec 17, 2021

Hi @clebergnu, overall, I think this is indeed an improvement to mitigate the pain of the coupling between the development branch of the two projects that we suffered on both sides.

There are only few points I was thinking about:

  1. If some compatibility issues were detected by the CI check with the latest Avocado, what action would be done by the VT developers/maintainers for the next step?

    • Report the issues to Avocado maintainers, then wait for the fixes or fix the issues by themselves, as usual.
    • Just ignore that until the next adaptation window.
    • Or something else.
  2. Whether a decoupling between the test provider repos and Avocado is something required as well.

    For instance, recently I am handling Root logger is not avilible to avocado tests after the 92.0 LTS autotest/tp-qemu#3048. And one thing came to my mind is, to avoid being affected by further compatibility issues, it's better to let VT be the API provider instead of Avocado for tests, which will make VT more like a middleware.

Anyway, please let me know your thoughts, thanks!

@clebergnu
Copy link
Contributor Author

Hi @clebergnu, overall, I think this is indeed an improvement to mitigate the pain of the coupling between the development branch of the two projects that we suffered on both sides.

There are only few points I was thinking about:

  1. If some compatibility issues were detected by the CI check with the latest Avocado, what action would be done by the VT developers/maintainers for the next step?

    • Report the issues to Avocado maintainers, then wait for the fixes or fix the issues by themselves, as usual.
    • Just ignore that until the next adaptation window.
    • Or something else.

I believe the immediate step would be to open issues on the Avocado repo ASAP, there's no need to wait for the adaptation window. During the adaptation windows, though, there would be an obligation to check for compatibility and have them addressed.

If it's an intentional change, there should be an explanation and a migration/workaround suggestion to be done on the Avocado-VT side as a response. If it's not an intentional change, thus a bug, there should be an acknowledgement and plan to fix it on the Avocado side.

  1. Whether a decoupling between the test provider repos and Avocado is something required as well.
    For instance, recently I am handling Root logger is not avilible to avocado tests after the 92.0 LTS autotest/tp-qemu#3048. And one thing came to my mind is, to avoid being affected by further compatibility issues, it's better to let VT be the API provider instead of Avocado for tests, which will make VT more like a middleware.

The test providers don't have formal tags/releases, so I find it hard to apply the same suggestions to them right now. IMO we should validate the model with Avocado and Avocado-VT, and them progress to include the test providers. There's little point IMO to have Avocado-VT and Avocado compatible without a compatible test provider.

Being more practical, I believe we should increase the coverage on CI by running real tests from real test providers. One example of what should be caught and addressed is in autotest/tp-qemu#3139.

With regards Avocado-VT being a middleware, IMO it already is in many aspects. IIUC your proposal is to limit the exposure of test providers to Avocado changes by using Avocado-VT as a shield. This is nice, intentionally-wise, but it comes at a cost of code duplication. Especially with the increase in upstream functional/integration tests, we can benefit from having one copy of a utility instead of the multiple and similar ones that can be found in Avocado, Avocado-VT and even on the test providers themselves.

This is a lengthier discussion, but a very important one! This has been discussed here, but more recently here. Your input is very much appreciated because Avocado-VT must be a part of the process, or else this effort will not be successful, but will just lead to the creation of another repo.

Anyway, please let me know your thoughts, thanks!

Thanks for the reply and I look forward to get this decided/resolved!

@chunfuwen
Copy link
Contributor

Hi @clebergnu, overall, I think this is indeed an improvement to mitigate the pain of the coupling between the development branch of the two projects that we suffered on both sides.
There are only few points I was thinking about:

  1. If some compatibility issues were detected by the CI check with the latest Avocado, what action would be done by the VT developers/maintainers for the next step?

    • Report the issues to Avocado maintainers, then wait for the fixes or fix the issues by themselves, as usual.
    • Just ignore that until the next adaptation window.
    • Or something else.

I believe the immediate step would be to open issues on the Avocado repo ASAP, there's no need to wait for the adaptation window. During the adaptation windows, though, there would be an obligation to check for compatibility and have them addressed.

If it's an intentional change, there should be an explanation and a migration/workaround suggestion to be done on the Avocado-VT side as a response. If it's not an intentional change, thus a bug, there should be an acknowledgement and plan to fix it on the Avocado side.

@clebergnu , in terms of intentional change, can we have one head-up step to avocado-vt members before happening, such as create one issue in avocado-vt issue tab ?

  1. Whether a decoupling between the test provider repos and Avocado is something required as well.
    For instance, recently I am handling Root logger is not avilible to avocado tests after the 92.0 LTS autotest/tp-qemu#3048. And one thing came to my mind is, to avoid being affected by further compatibility issues, it's better to let VT be the API provider instead of Avocado for tests, which will make VT more like a middleware.

The test providers don't have formal tags/releases, so I find it hard to apply the same suggestions to them right now. IMO we should validate the model with Avocado and Avocado-VT, and them progress to include the test providers. There's little point IMO to have Avocado-VT and Avocado compatible without a compatible test provider.

Being more practical, I believe we should increase the coverage on CI by running real tests from real test providers. One example of what should be caught and addressed is in autotest/tp-qemu#3139.

With regards Avocado-VT being a middleware, IMO it already is in many aspects. IIUC your proposal is to limit the exposure of test providers to Avocado changes by using Avocado-VT as a shield. This is nice, intentionally-wise, but it comes at a cost of code duplication. Especially with the increase in upstream functional/integration tests, we can benefit from having one copy of a utility instead of the multiple and similar ones that can be found in Avocado, Avocado-VT and even on the test providers themselves.

This is a lengthier discussion, but a very important one! This has been discussed here, but more recently here. Your input is very much appreciated because Avocado-VT must be a part of the process, or else this effort will not be successful, but will just lead to the creation of another repo.

Anyway, please let me know your thoughts, thanks!

Thanks for the reply and I look forward to get this decided/resolved!

@beraldoleal
Copy link
Member

@clebergnu any updates here?

@clebergnu
Copy link
Contributor Author

Hi @clebergnu, overall, I think this is indeed an improvement to mitigate the pain of the coupling between the development branch of the two projects that we suffered on both sides.
There are only few points I was thinking about:

  1. If some compatibility issues were detected by the CI check with the latest Avocado, what action would be done by the VT developers/maintainers for the next step?

    • Report the issues to Avocado maintainers, then wait for the fixes or fix the issues by themselves, as usual.
    • Just ignore that until the next adaptation window.
    • Or something else.

I believe the immediate step would be to open issues on the Avocado repo ASAP, there's no need to wait for the adaptation window. During the adaptation windows, though, there would be an obligation to check for compatibility and have them addressed.
If it's an intentional change, there should be an explanation and a migration/workaround suggestion to be done on the Avocado-VT side as a response. If it's not an intentional change, thus a bug, there should be an acknowledgement and plan to fix it on the Avocado side.

@clebergnu , in terms of intentional change, can we have one head-up step to avocado-vt members before happening, such as create one issue in avocado-vt issue tab ?

Of course.

I guess the best way is to try to label issues/PRs on Avocado that are known to cause compatibility issues. This can maybe be proved by a GH Actions that we can run on-demand. @ana do you think that's something easy to do?

Then, we can tag Avocado-VT developers, but it'd be nice to have a way to know which avocado-vt member to tag. Should it be yourself (@chunfuwen) and maybe @luckyh and @richtja .

If the Avocado-VT member list is not a simple static list, then we can follow what other projects, such as QEMU do. They have a clear mapping of MAINTAINERS (in a file with the same name) that maybe we could leverage here too.

  1. Whether a decoupling between the test provider repos and Avocado is something required as well.
    For instance, recently I am handling Root logger is not avilible to avocado tests after the 92.0 LTS autotest/tp-qemu#3048. And one thing came to my mind is, to avoid being affected by further compatibility issues, it's better to let VT be the API provider instead of Avocado for tests, which will make VT more like a middleware.

The test providers don't have formal tags/releases, so I find it hard to apply the same suggestions to them right now. IMO we should validate the model with Avocado and Avocado-VT, and them progress to include the test providers. There's little point IMO to have Avocado-VT and Avocado compatible without a compatible test provider.
Being more practical, I believe we should increase the coverage on CI by running real tests from real test providers. One example of what should be caught and addressed is in autotest/tp-qemu#3139.
With regards Avocado-VT being a middleware, IMO it already is in many aspects. IIUC your proposal is to limit the exposure of test providers to Avocado changes by using Avocado-VT as a shield. This is nice, intentionally-wise, but it comes at a cost of code duplication. Especially with the increase in upstream functional/integration tests, we can benefit from having one copy of a utility instead of the multiple and similar ones that can be found in Avocado, Avocado-VT and even on the test providers themselves.
This is a lengthier discussion, but a very important one! This has been discussed here, but more recently here. Your input is very much appreciated because Avocado-VT must be a part of the process, or else this effort will not be successful, but will just lead to the creation of another repo.

Anyway, please let me know your thoughts, thanks!

Thanks for the reply and I look forward to get this decided/resolved!

@chunfuwen
Copy link
Contributor

@clebergnu
@dzhengfy
dzhengfy sent out one mail to hopefully have one virtual meeting to learn more details before New Year, please have a check and see how we can go on.

@ana
Copy link
Contributor

ana commented Jan 18, 2022

I guess the best way is to try to label issues/PRs on Avocado that are known to cause compatibility issues. This can maybe be proved by a GH Actions that we can run on-demand. @ana do you think that's something easy to do?

Yeah, although it'll take a few iterations to have something good.

@clebergnu
Copy link
Contributor Author

@clebergnu @dzhengfy dzhengfy sent out one mail to hopefully have one virtual meeting to learn more details before New Year, please have a check and see how we can go on.

@chunfuwen my apologies but I missed that email (and I was on vacation) during that time. I'll resurrect the virtual meeting arrangements, but before, I'll summarize the points discussed here and add them to the Development/Contributing related docs in a V2 of this PR.

@dzhengfy
Copy link
Contributor

@clebergnu @dzhengfy dzhengfy sent out one mail to hopefully have one virtual meeting to learn more details before New Year, please have a check and see how we can go on.

@chunfuwen my apologies but I missed that email (and I was on vacation) during that time. I'll resurrect the virtual meeting arrangements, but before, I'll summarize the points discussed here and add them to the Development/Contributing related docs in a V2 of this PR.

@clebergnu , no problem. I resent my email to you today and a document link was also included. In that document, I listed some questions. It is appreciated if you have a look :D

@luckyh
Copy link
Contributor

luckyh commented Jan 19, 2022

Thanks for the reply and I look forward to get this decided/resolved!

@clebergnu Sorry for the delayed response, for some reason I will give the full reply later.

And I'm going to approve this PR first, since I'd support the decoupling we are talking about, thanks!


avocado_devel_task:
container:
image: quay.io/avocado-framework/avocado-vt-ci-fedora-34
Copy link
Contributor

Choose a reason for hiding this comment

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

do you mind using quay.io/avocado-framework/avocado-vt-ci-fedora-35 instead?
This image is already available in quay.

Copy link
Contributor

@ana ana left a comment

Choose a reason for hiding this comment

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

Other than the small comment I left above, this looks good to me.

Copy link
Contributor

@pevogam pevogam left a comment

Choose a reason for hiding this comment

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

Hi @clebergnu, my general concern remains in the fact that our CI which is configured to continuously download and test the most recent Avocado and Avocado VT sources will usually get stuck for long periods when there is an incompatibility that no one is willing to fix for weeks but this PR looks good to me and we can examine how much this is the case later on. At present it helps us quickly isolate changes that caused the incompatibility and if something is constantly broken I will have to spend too much effort in disabling and/or circumventing it and testing only parts of the integration. Perhaps later on some of my our reports can also help improve the automated compatibility checks you started, let's see.

@clebergnu
Copy link
Contributor Author

Given that we have 4 approvals here, I'm going to merge this and send the suggestions and documentation on top of this. Thanks a lot for everyone that contributed to the discussion!

@clebergnu clebergnu merged commit e572b85 into avocado-framework:master Jan 19, 2022
@dzhengfy
Copy link
Contributor

@clebergnu hi, cleber, as we talked in planning meeting, what's the status to add more tp-libvirt/tp-qemu tests in avocado-vt CI job?

clebergnu added a commit to clebergnu/avocado that referenced this pull request May 11, 2022
The lockstep approach of compatibility between Avocado official RPM
builds (the ones in the Fedora distros), is no longer possible due to
the fact that only some Avocado releases will be guarantee
compatibility with Avocado-VT.

During the development cycle thta will result in Avocado and
Avocado-VT 98.0, compatibility will be restored.  Thus, this check
will be re-enabled for specific versions.

Reference: avocado-framework/avocado-vt#3295
Signed-off-by: Cleber Rosa <[email protected]>
clebergnu added a commit to clebergnu/avocado that referenced this pull request May 16, 2022
The lockstep approach of compatibility between Avocado official RPM
builds (the ones in the Fedora distros), is no longer possible due to
the fact that only some Avocado releases will be guarantee
compatibility with Avocado-VT.

During the development cycle that will result in Avocado and
Avocado-VT 98.0, compatibility will be restored.  Thus, this check
will be re-enabled for specific versions.

Reference: avocado-framework/avocado-vt#3295
Signed-off-by: Cleber Rosa <[email protected]>
@clebergnu
Copy link
Contributor Author

@clebergnu hi, cleber, as we talked in planning meeting, what's the status to add more tp-libvirt/tp-qemu tests in avocado-vt CI job?

Hi @dzhengfy ,

This is a very late reply, but please see #3458. It was a bit hard to find another two tp-qemu tests that would run quickly enough for the CI environment. Running tp-libvirt tests seems to be harder, given that the environment is container based, so bridged networking is not available (among other things).

Feel free to propose new tests, and/or propose a way to run tp-libvirt based tests.

Regards!

@pevogam
Copy link
Contributor

pevogam commented Jul 15, 2022

@clebergnu hi, cleber, as we talked in planning meeting, what's the status to add more tp-libvirt/tp-qemu tests in avocado-vt CI job?

Hi @dzhengfy ,

This is a very late reply, but please see #3458. It was a bit hard to find another two tp-qemu tests that would run quickly enough for the CI environment. Running tp-libvirt tests seems to be harder, given that the environment is container based, so bridged networking is not available (among other things).

Feel free to propose new tests, and/or propose a way to run tp-libvirt based tests.

Regards!

@clebergnu I guess by this you mean limitations in the CI environment specifically on Github because the LXC containers from the corresponding spawner PR in Avocado avocado-framework/avocado#4158 do support bridged networking within each container and thus vms residing within it.

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.

7 participants