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

Bumping fabric8 k8s client to 6.7.2 and adjusting OpenShift class accordingly #1

Merged

Conversation

fabiobrz
Copy link

@fabiobrz fabiobrz commented Jul 21, 2023

Hi @RoyalKarma - a draft for a potential solution about what we discussed yesterday.
This is just a first stab, and I am going to play/test a little bit more with it, since I havent tried at all it yet.
I'll keep you posted.

Please make sure your PR meets the following requirements:

  • Pull Request contains a description of the changes
  • Pull Request does not include fixes for multiple issues/topics
  • Code is formatted, imports ordered, code compiles and tests are passing
  • Code is self-descriptive and/or documented

@fabiobrz
Copy link
Author

I improved it by using the Client adapter approach which is already designed and used in the kubernetes client, see https://github.com/fabric8io/kubernetes-client/blob/v6.7.2/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/extension/ClientAdapter.java and https://github.com/fabric8io/kubernetes-client/blob/v6.7.2/openshift-client-api/src/main/java/io/fabric8/openshift/client/NamespacedOpenShiftClientAdapter.java

mvn clean install is now green including local XTF tests.

Next step I'll run some integration tests from Intersmash.

@RoyalKarma
Copy link
Owner

Hi, thank you, I've played around with your changes, tests are still mostly failing with

Failure executing: GET at: . Message: pods is forbidden: User "" cannot list resource "pods" in API group "" in the namespace "8". Received status: Status(apiVersion=v1, code=403, details=StatusDetails(causes=[], group=null, kind=pods, name=null, retryAfterSeconds=null, uid=null, additionalProperties={}), kind=Status, message=pods is forbidden: User "" cannot list resource "pods" in API group "" in the namespace "80-openjdk17-o7m4", metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=Forbidden, status=Failure, additionalProperties={}).

Last time I had this issue, it was caused by missing .admin() in a test, but I'm not sure why its an issue now. When I run the tests locally with the admin token set, they pass fine so I think this is the correct solution.
Also version 6.8.0 of the client released, but it doesnt seem to break anything this time.

@fabiobrz
Copy link
Author

Hi, thank you, I've played around with your changes, tests are still mostly failing with

Failure executing: GET at: . Message: pods is forbidden: User "" cannot list resource "pods" in API group "" in the namespace "8". Received status: Status(apiVersion=v1, code=403, details=StatusDetails(causes=[], group=null, kind=pods, name=null, retryAfterSeconds=null, uid=null, additionalProperties={}), kind=Status, message=pods is forbidden: User "" cannot list resource "pods" in API group "" in the namespace "80-openjdk17-o7m4", metadata=ListMeta(_continue=null, remainingItemCount=null, resourceVersion=null, selfLink=null, additionalProperties={}), reason=Forbidden, status=Failure, additionalProperties={}).

I don't know what this issue is about. From the details it looks to me as you're testing the changes through the EAP QE OpenShift TS. I am doing the same with Intersmash, and it seems to work (except for some failures I still need to fix) . Have you tried referencing the XTF SNAPSHOT based on our cumulative changes that I released a couple of days ago?

@RoyalKarma
Copy link
Owner

Yes, I'm using the new snapshot, but still no luck with the errors. I don't see anything in the client changelog that could be causing this.

@simkam
Copy link

simkam commented Aug 22, 2023

I sent an upgrade to 6.8.1 to @fabiobrz 's branch. It includes the fix that was causing problems in OpenShift TS - commented-out token matching. @RoyalKarma When merged please run full tests again.

fabiobrz#1

upgrade to 6.8.1 + fix token matching
@RoyalKarma RoyalKarma merged commit bf5cd0c into RoyalKarma:update_kn_client Aug 24, 2023
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.

3 participants