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

[#456][#462] refactor logic determining cluster version and logic downloading oc binar #465

Merged
merged 2 commits into from
Nov 4, 2021

Conversation

simkam
Copy link
Contributor

@simkam simkam commented Oct 22, 2021

fixes #456
fixes #462

  • cache openshift version (loaded either from configuration or from cluster)
  • download oc binary from
    • mirror.openshift.com (if cluster version is known)
    • cluster (if version is uknown assume OCP4 and download it from cluster)o
  • binary download supports stable (default), latest, candidate and fast channels if configured version is in major.minor format only

Some public methods from OpenShifts class are marked as deprecated, they aren't now used internally by XTF and I believe they shouldn't have been public.

Some tests are included, the rest (major.minor only version, no version, no permissions to detect cluster version ...) tested manually.

…nd logic downloading oc binary

fixes xtf-cz#456
fixes xtf-cz#462

- cache openshift version (loaded either from configuration or from cluster)
- download oc binary from
  - mirror.openshift.com (if cluster version is known)
  - cluster (if version is uknown assume OCP4 and download it from cluster)o
- binary download supports stable (default), latest, candidate and fast channels if configured version is in major.minor format only

Some public methods from `OpenShifts` class are marked as deprecated, they aren't now use internally by XTF and I believe they shouldn't have been public.
…erBinder"' during tests

SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
@simkam
Copy link
Contributor Author

simkam commented Oct 22, 2021

@mcarlett you might want to review this as well since you added binary cache feature. I'm changing it a lot, but it should work exactly the same way. I have marked public caching methods in OpenShifts class as deprecated, they are no longer used internally and I believe they should have never been public. Let me know if it is an issue. Thanks

@mcarlett
Copy link
Contributor

@mcarlett you might want to review this as well since you added binary cache feature. I'm changing it a lot, but it should work exactly the same way. I have marked public caching methods in OpenShifts class as deprecated, they are no longer used internally and I believe they should have never been public. Let me know if it is an issue. Thanks

@simkam thanks for the refactoring, I think we can remove deprecated methods and dedicated tests as well. Moreover, you can remove the unused deprecated constants in OpenShifts (moved into ClusterVersionOpenShiftBinaryPathResolver)

@simkam
Copy link
Contributor Author

simkam commented Oct 27, 2021

Thanks for the review. Since those methods are public and they were in the last release, I would keep them deprecated for few releases before we remove them. I know there is low chance that somebody is using them, but it is a best practice after all.

@mnovak1 mnovak1 merged commit 7882fd1 into xtf-cz:master Nov 4, 2021
@simkam simkam deleted the version branch November 23, 2021 13:45
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.

xtf.openshift.version should take x.y format again review logic getting OCP version
3 participants