Skip to content

mock: generation is an Integer field #3936

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

Merged
merged 2 commits into from
Mar 8, 2022

Conversation

petitlapin
Copy link
Contributor

@petitlapin petitlapin commented Mar 5, 2022

Description

The generation field in the Object Metadata should be an Integer (https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.19/#objectmeta-v1-meta).
I'm using the Mock to test the api from https://github.com/kubernetes-client/c and it fails to parse the generation field because it is actually a string.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

Sorry, something went wrong.

@petitlapin
Copy link
Contributor Author

Hi,

I can open an issue to reference if needed. Also, in the template, the CHANGELOG should refer to https://github.com/fabric8io/kubernetes-client/blob/master/CHANGELOG.md, not https://github.com/fabric8io/kubernetes-client/CHANGELOG.md

I ran the compilation and found no bug with the existing tests.

@petitlapin petitlapin marked this pull request as ready for review March 5, 2022 13:05
@petitlapin petitlapin changed the title Draft: mock: generation is an Integer field mock: generation is an Integer field Mar 5, 2022
@petitlapin petitlapin force-pushed the work/generation-mock-integer branch from ee1a7e9 to 67ccc66 Compare March 5, 2022 18:37
@petitlapin
Copy link
Contributor Author

petitlapin commented Mar 5, 2022

I reformated the whole file to follow the code style. The only change in this PR is:

    metadata.put(GENERATION, getOrDefault(existingMetadata, GENERATION, "1"));

to

    metadata.put(GENERATION, Integer.parseInt(getOrDefault(existingMetadata, GENERATION, "1")));

@shawkins shawkins requested review from shawkins and manusa March 7, 2022 16:56
@shawkins shawkins added this to the 6.0.0 milestone Mar 7, 2022
@manusa manusa force-pushed the work/generation-mock-integer branch from 67ccc66 to 610d8b7 Compare March 8, 2022 11:46
@manusa
Copy link
Member

manusa commented Mar 8, 2022

I'm using the Mock to test the api from https://github.com/kubernetes-client/c and it fails to parse the generation field because it is actually a string.

This is very interesting, is your setup available or documented somewhere?

We had similar request a while back: #1601

Signed-off-by: Marc Nuri <marc@marcnuri.com>
Copy link
Contributor

@shawkins shawkins left a comment

Choose a reason for hiding this comment

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

LGTM

@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 8, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@manusa manusa merged commit f7cdbd6 into fabric8io:master Mar 8, 2022
@petitlapin
Copy link
Contributor Author

petitlapin commented Mar 8, 2022

I'm using the Mock to test the api from https://github.com/kubernetes-client/c and it fails to parse the generation field because it is actually a string.

This is very interesting, is your setup available or documented somewhere?

We had similar request a while back: #1601

Unfortunately, I can't share much info/code as it is done on closed source (I'm not a developer of the kubernetes-c library).
I'm doing 2 small librairies to create and list services in both Java and C++.
For the C++ part, I'm using the kubernetes-c library to do so.
To unit test, I've created a small Java executable that starts the mock in CRUD mode and I'm connecting to it to create and list services (using boost in the c++ part to start the java executable and stop it at the end of the test).

Creating the services works fine but when listing them, the parser used in kubernetes-c is strict and did not allow the wrong type so a part of the metadata was not retrieved correctly (and there was some leaks too but that's another issue no the C library).

Thank you for the quick integration of the fix!

@petitlapin petitlapin deleted the work/generation-mock-integer branch March 8, 2022 17:29
@manusa
Copy link
Member

manusa commented Mar 9, 2022

To unit test, I've created a small Java executable that starts the mock in CRUD mode and I'm connecting to it to create and list services (using boost in the c++ part to start the java executable and stop it at the end of the test).

Thanks for sharing.

This is the part I was interested on, thanks for sharing. So if I understood correctly, basically your testing process runs a separate JVM process with a dummy MockServer and probably prints the HTTP port. The C++ unit test process orchestrates this part, picks up the port, runs the tests, and finally stops the JVM process.

If I may, why did you choose to use Fabric8 for this purpose instead of Microcks or other alternatives?

@petitlapin
Copy link
Contributor Author

This is the part I was interested on, thanks for sharing. So if I understood correctly, basically your testing process runs a separate JVM process with a dummy MockServer and probably prints the HTTP port. The C++ unit test process orchestrates this part, picks up the port, runs the tests, and finally stops the JVM process.

The port is hardcoded on both sides (using the same KUBERNETES_MASTER env variable), the ip is set to localhost so no need to have a cluster running, but yes, that's the principle.

If I may, why did you choose to use Fabric8 for this purpose instead of Microcks or other alternatives?

I'm not the one who did the research on the tool, but I guess we are already using the library so it's the same repo and one less tool to install aside :). And this one was easy to set up and use directly.

@manusa
Copy link
Member

manusa commented Mar 9, 2022

Jut one last question 😅, so you mind if I share this thread publicly?

@petitlapin
Copy link
Contributor Author

I'm not sure by what you mean publicly, but this discussion is already here so yes, you can share what I wrote here :)

@manusa manusa added the 5.12.x Backportable tentative label Mar 9, 2022
@manusa manusa removed the 5.12.x Backportable tentative label Apr 5, 2022
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.

None yet

3 participants