Skip to content
New issue

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

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

Already on GitHub? # to your account

[MAINTENANCE] Move functionality from libs to the testsuite repo where meaningful #2219

Open
martin-mat opened this issue Feb 19, 2025 · 3 comments

Comments

@martin-mat
Copy link
Collaborator

martin-mat commented Feb 19, 2025

Currently, testsuite functionality uses set of crystal repos:

  • cnf-testsuite/tar
  • cnf-testsuite/find - will not be migrated and instead become a part of tar module as find is only used in tar.
  • cnf-testsuite/git
  • cnf-testsuite/docker_client
  • cnf-testsuite/kubectl_client
  • cnf-testsuite/cluster_tools
  • cnf-testsuite/kernel_introspection
  • cnf-testsuite/k8s_kernel_introspection
  • cnf-testsuite/helm
  • cnf-testsuite/k8s_netstat
  • cnf-testsuite/release_manager

In many cases, the functionality inside them is quite testsuite-specific and not really universally usable, or does not contain enough of meaningful logic/functionality that would justify a separate library for it.
Moving the functionality to the testsuite repo would bring benefits:

  • much simplified testing of changes using gh actions in the testsuite
  • the libs are even if a different github organization, so moving them woul make issue tracking and controlling easier

Actions:

  • identify libraries whose functionality would be meaningful to move to the testsuite
  • move the functionality to the testsuite while keeping a good core organization
  • mark the library as deprecated
@collivier
Copy link
Collaborator

lgtm ! it would defacto fix the lib version mismatches

We might also consider removing unused or useless code and it would be great to remove the subprocesses as much as we can (see kubectl as a good example where API is rather used).

@rafal-lal
Copy link
Collaborator

lgtm ! it would defacto fix the lib version mismatches

We might also consider removing unused or useless code and it would be great to remove the subprocesses as much as we can (see kubectl as a good example where API is rather used).

This has been started already in cnf-testsuite/kubectl_client#17 and cnf-testsuite/helm#6. These PRs were started as part of #2199 but since I had the chance to go through all the code anyway I also deleted much of the unused / useless stuff and tried to clean the rest as much as possible. Unfortunately these PRs are quite big and there is a lot to review.

@svteb
Copy link
Collaborator

svteb commented Feb 26, 2025

I think we should discuss the strategy a bit more:

I propose that we move all cnf-testsuite org shards to the main repo without exception. There is really no reason not to, as over half of the shards are inter-connected and those that are not (tar, find, git, ...) could likely be replaced by an external shard in the future (or just moved to utils), thus further reducing our burden.

  1. Naming considerations
  • We will still have some external shards (halite, totem, ...), meaning that the lib directory is here to stay. I think that we could move the internal libraries into src/internal_lib(s), moving them all to utils would bloat an already bloated bottom-level directory even more.
  • Optionally we could create an internal_lib(s) directory in the root of the project but that seems a little counter-intuitive since there will be a lib and internal_lib(s) directory at the same level. Although this could make require-ing in spec tests a little more easier (more on this later).
  1. Moving the shards
  • Once we decide where to move the shards, we should use the git subtree command to integrate the repos with their commit history (optionally) intact, for example:
git subtree add --prefix=src/internal_lib/kubectl_client https://github.com/cnf-testsuite/kubectl_client.git main
;--squash - optional parameter to reduce the commit history to just one
  • (Speculatory) When this is done the next order of operations should be:
    2.1. Delete .gitignore, shard.yaml and shard.lock files. As for the .github directory that some shards have, we might want to retain it or just outright delete it. I can see a future where we would move the contents of these to the root .github and edit the main actions file to run these as dependencies, but it might be simpler to copy the spec test files of the shards into the spec directory and be done with it.
    2.2. Remove the shard from the shard.yaml of main project.
    2.3. There will likely be some kind of unresolved dependency error, the easiest solution will probably be to add the core module of the shard in question to the require-ments list of src/tasks/utils/utils.cr as this file is used in nearly every src directory file.
    2.4. Many spec tests will not work as well, but a similar spec/utils/utils_spec.cr file exists that we could populate with require statements to ease the process.
    2.5. Last but not least, even the internal libraries themself will need require-s to be resolved, although this should not be too hard as all the require-s will basically change from require ABC to require ../abc/ABC (they should all be on the same level after all).
    2.5. Run crystal build src/cnf-testsuite.cr and hope for the best.
    2.6. Resolve the errors as they come, then push the changes.
  • We should consider splitting this into at least two pull requests, one where all the shards are moved to the internal_lib(s) directory and possibly stripped of .gitignore, shard.yaml and shard.lock, (this should not cause any conflicts with existing shard configuration). The other pull request would be about resolving the dependency errors, this one could be split even further into the "easier" one with simple shards like git, find, tar where not many changes are anticipated and a harder one with complicated dependencies like kubectl_client and helm.
  • Optionally we could have a test run where we attempt to move one simple shard and see what other challenges could come up.
  1. Bureaucracy
  • Mark the shards as deprecated.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
Status: To do
Development

No branches or pull requests

4 participants