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

VersionInfo contains null data in OpenShift 4.6 #3189

Closed
vsevel opened this issue May 28, 2021 · 8 comments · Fixed by #3198
Closed

VersionInfo contains null data in OpenShift 4.6 #3189

vsevel opened this issue May 28, 2021 · 8 comments · Fixed by #3198
Assignees
Milestone

Comments

@vsevel
Copy link

vsevel commented May 28, 2021

This issue has been found in 5.4.0, which is used in Quarkus 2.0.0.CR1. And it is a regression compared to 5.3.1, which was used in Quarkus 2.0.0.Alpha3.

With the following code:

    @Inject
    @Privileged
    OpenShiftClient privilegedClient;
...
            VersionInfo version = privilegedClient.getVersion();
            String major = version.getMajor();
            log.info("Version Major " + major);
            log.info("Version Minor " + version.getMinor());
            log.info("Version BuildDate " + version.getBuildDate());

In 5.3.1 I see:

2021-04-30 17:21:55,966 INFO  [com.x.arte.ocpdeploy.deploy.ContextMgr] (main) Version Major 4
2021-04-30 17:21:56,011 INFO  [com.x.arte.ocpdeploy.deploy.ContextMgr] (main) Version Minor 6.13
2021-04-30 17:21:56,012 INFO  [com.x.arte.ocpdeploy.deploy.ContextMgr] (main) Version BuildDate Tue Feb 02 14:55:04 GMT 2021

In 5.4.0 I see:

2021-05-28 19:46:36,567 INFO  [com.x.arte.ocpdeploy.deploy.ContextMgr] (main) Version Major null
2021-05-28 19:46:36,568 INFO  [com.x.arte.ocpdeploy.deploy.ContextMgr] (main) Version Minor null
2021-05-28 19:46:36,568 INFO  [com.x.arte.ocpdeploy.deploy.ContextMgr] (main) Version BuildDate null
@matthyx
Copy link
Contributor

matthyx commented May 31, 2021

possibly fixed by #3175 ?

@rohanKanojia
Copy link
Member

Not 100% sure since #3175 seems to fix non release OpenShift versions 4.8.0-0.nightly-2021-05-26-071911

@matthyx
Copy link
Contributor

matthyx commented May 31, 2021

I have started a huge comparison between 5.3.1 and 5.4.0 trying to see what's changed regarding the versions but no success so far :(

@rohanKanojia
Copy link
Member

ohk, I tried reproducing this and I was able to reproduce this. it's happening on my 4.7.2 cluster as well

@matthyx
Copy link
Contributor

matthyx commented Jun 1, 2021

ohk, I tried reproducing this and I was able to reproduce this. it's happening on my 4.7.2 cluster as well

Awesome, did you try with master as well ?

At first I thought it was permission issues with the service account, but I'm able to curl "apis/config.openshift.io/v1/clusterversions" within a container... and of course unit tests are working well.

It's not so easy for me to play with different versions, as our app now uses Quarkus and it's a bit of #dependencyhell to force the framework using my custom build.

@rohanKanojia rohanKanojia self-assigned this Jun 1, 2021
@rohanKanojia
Copy link
Member

I think this behavior was introduced by #3049 .

We seem to have minor problem in our code which was exposed by this PR. We are calculating openshift 4 version as a fallback when old openshift version apicall fails:

try {
// Try to get version using OpenShift 3 URLs.
// Method will throw Exception if it failed
return super.fetchVersion();
} catch (KubernetesClientException | NullPointerException exception) {
try {
// Handle Openshift 4 version case
return fetchOpenshift4Version();

However, in ClusterOperationsImpl we're not checking the response code or whether our response was even successful before assigning to map. Earlier since there was no null check so code used to throw NPE in case of a 404 response body since it won't contain Version data and that NPE was caught by OpenShiftClusterOperationsImpl

@matthyx
Copy link
Contributor

matthyx commented Jun 1, 2021

ahah, I knew it was related to NPE somehow, just couldn't verify it, good job @rohanKanojia !!!!!

Will you be able to include this as well into the patch release 5.4.1 in time for Quarkus release, please?

rohanKanojia added a commit to rohanKanojia/kubernetes-client that referenced this issue Jun 1, 2021
assert response code after getting response from apiServer rather than
depending upon fragile null check
rohanKanojia added a commit to rohanKanojia/kubernetes-client that referenced this issue Jun 1, 2021
assert response code after getting response from apiServer rather than
depending upon fragile null check
manusa pushed a commit that referenced this issue Jun 1, 2021
assert response code after getting response from apiServer rather than
depending upon fragile null check
manusa pushed a commit that referenced this issue Jun 1, 2021
assert response code after getting response from apiServer rather than
depending upon fragile null check

(cherry picked from commit 062313c)
@manusa
Copy link
Member

manusa commented Jun 1, 2021

Will you be able to include this as well into the patch release 5.4.1 in time for Quarkus release, please?

quarkusio/quarkus#17598

@manusa manusa added this to the 5.4.1 milestone Jun 9, 2021
adietish added a commit to adietish/intellij-common that referenced this issue Sep 9, 2021
adietish added a commit to adietish/intellij-openshift-connector that referenced this issue Sep 9, 2021
adietish added a commit to adietish/intellij-tekton that referenced this issue Sep 9, 2021
adietish added a commit to adietish/intellij-tekton that referenced this issue Sep 9, 2021
adietish added a commit to adietish/intellij-tekton that referenced this issue Sep 9, 2021
adietish added a commit to redhat-developer/intellij-common that referenced this issue Sep 10, 2021
adietish added a commit to adietish/intellij-openshift-connector that referenced this issue Sep 30, 2021
adietish added a commit to redhat-developer/intellij-openshift-connector that referenced this issue Oct 1, 2021
 (#351)

* use kubernetes-client 5.7.2 to fix fabric8io/kubernetes-client#3189
* use intellij-common:1.3.0 with same kubernetes-client
* depend on intellij-kubernetes with the same kubernetes-client
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 a pull request may close this issue.

4 participants