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

Add missing API calls for Cloudstack 4.20 #91

Merged
merged 4 commits into from
Dec 17, 2024
Merged

Conversation

phsm
Copy link
Contributor

@phsm phsm commented Nov 5, 2024

Fixes #22

I've noticed that a lot of API calls are missing from the cloudstack/ directory.

Thus, I took listApis output from the latest Cloudstack version at the time of writing this, 4.20.0.0, and added all those missing calls to the layout.go.

There are 2 commits in the PR:

  • One updates listApis to the Cloudstack version 4.20.0. For some reason several API calls were not present in my listApis output, namely:
listVmwareDcVms
addVmwareDc
listVmwareDcs
removeVmwareDc
updateVmwareDc
addKubernetesSupportedVersion
createKubernetesCluster
deleteKubernetesCluster
deleteKubernetesSupportedVersion
getKubernetesClusterConfig
listKubernetesClusters
listKubernetesSupportedVersions
scaleKubernetesCluster
startKubernetesCluster
stopKubernetesCluster
updateKubernetesSupportedVersion
upgradeKubernetesCluster
addVirtualMachinesToKubernetesCluster
removeVirtualMachinesFromKubernetesCluster
createServiceInstance
cloudianSsoLogin

I have ported them from listApis.json from the commit d4d7f16, except cloudianSsoLogin which I could not find anywhere. Therefore, I've removed this call from layout.go.

This commit also includes changes to layout.go that introduces new API calls, and also moves some Netscaler-related API calls to the new Service NetscalerService.

  • The second commit contains all the files that make all generated

There were no errors/warnings during the make all.

Copy link
Contributor

@Pearl1594 Pearl1594 left a comment

Choose a reason for hiding this comment

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

code built successfully. code LGTM

@DaanHoogland
Copy link
Contributor

@phsm I merged some PRs and now you are stuck with a lot of conflicts. So for I can see these are not complex, just a lot. Can you have a look, please?

@phsm
Copy link
Contributor Author

phsm commented Dec 13, 2024

I see you've merged the previous PRs implementing the slices of API calls that had been emerged at different CS versions.

This may as well render my PR obsolete.

Later I will check the build against CS 4.20 listApis output (thankfully the code generator emits warnings when it encounters an unknown API call), and will adjust my PR to have what is missing.

@phsm phsm reopened this Dec 16, 2024
@phsm phsm changed the title Add missing API calls Add missing API calls for Cloudstack 4.20 Dec 16, 2024
@phsm
Copy link
Contributor Author

phsm commented Dec 16, 2024

@DaanHoogland updated both the code, and the PR header/description.

Comment on lines -793 to -795
"ShutdownService": {
"readyForShutdown",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

any specific reason to remove this? @phsm as the generate.go file has a reference to this api, causing issues:

test/ShutdownService_test.go:42:15: client.Shutdown undefined (type *cloudstack.CloudStackClient has no field or method Shutdown)
test/ShutdownService_test.go:43:20: client.Shutdown undefined (type *cloudstack.CloudStackClient has no field or method Shutdown)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason was to make the layout structure to follow the structure from here: https://cloudstack.apache.org/api/apidocs-4.20/

The "readyForShutdown" API call looks to be a part of ManagementService now.

@Pearl1594
Copy link
Contributor

For some reason, the tests seem to fail on this PR - @phsm can you please help us with this:
I needed to make the following changes to make it work - I am not sure why the 1st two changes are required, as it isn't required upstream:

diff --git a/Makefile b/Makefile
index 015bba8..33ad3ea 100644
--- a/Makefile
+++ b/Makefile
@@ -27,4 +27,4 @@ test:
 
 MOCKGEN := mockgen
 mockgen: ## Download conversion-gen locally if necessary.
-       go get github.com/golang/mock/mockgen; go install github.com/golang/mock/mockgen;
+       go get go.uber.org/mock/mockgen; go install go.uber.org/mock/mockgen;

examples/mock_test.go file


diff --git a/examples/mock_test.go b/examples/mock_test.go
index b965c3a..da50adb 100644
--- a/examples/mock_test.go
+++ b/examples/mock_test.go
@@ -23,7 +23,7 @@ import (
        "testing"
 
        "github.com/apache/cloudstack-go/v2/cloudstack"
-       "github.com/golang/mock/gomock"
+       "go.uber.org/mock/gomock"
 )
 
 func Test_Mock(t *testing.T) {

layout.go

git diff generate/layout.go
diff --git a/generate/layout.go b/generate/layout.go
index d399ba9..2271abb 100644
--- a/generate/layout.go
+++ b/generate/layout.go
@@ -864,6 +864,9 @@ var layout = apiInfo{
        "RegistrationService": {
                "registerOauthProvider",
        },
+       "ShutdownService": {
+               "readyForShutdown",
+       },
        "DiagnosticsService": {
                "getDiagnosticsData",
                "runDiagnostics",

@phsm
Copy link
Contributor Author

phsm commented Dec 16, 2024

@Pearl1594 This was to tidy up an abandoned https://github.com/golang/mock/ package.
The authors there suggest to switch to a maintained fork instead.

I replaced the rest of the old package references with go.uber.org/mock/gomock, lets see if the tests will work now.

As for the "ShutdownService" removal: as I mentioned before, the intention was to keep the layout.go consistent with the API calls reference page.

@Pearl1594
Copy link
Contributor

Thanks @phsm - tests now run successfully! Thank you 👍

@DaanHoogland DaanHoogland merged commit d654fcc into apache:main Dec 17, 2024
1 check passed
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.

Missing API support
3 participants