-
Notifications
You must be signed in to change notification settings - Fork 114
Split/Duplicate ARO-HCP API models #1100
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
Conversation
We achieve that by removing any referencing via the ref statement in the models/aro_hcp definitions. We then copy all the attributes for the resources we have defined in the existing aro-hcp resources from model/clusters_mgmt, even if not currently used by ARO-HCP. However, we remove any locator statements to avoid providing additional endpoints under the aro_hcp/v1alpha1 API URL for the existing resources defined in aro-hcp. The copying of all elements is needed to resolve all the attribute references now that we removed the referencing mechanism. It also ensures that there are no dependencies with ocm commercial from within the aro-hcp models. This commit does not copy resources that might be referenced by the copied attributes. This will be done in a follow-up commit. This will end up adding many data types in the generated Go models that won't be used and they will be available as data types. This will also modify existing methods of the generated Go types to use the duplicated data types. This can have an impact on the consumers of the models but for aro-hcp case we expect if they need to change it should not be a big amount of work.o
This will ensure all the data types from ocm commercial are now duplicated in aro-hcp. This means that the generated Go model will now include all the new types.
Now that we have duplicated all existing ocm commercial models that did not exist in aro-hcp models we regenerate the clientapi go data types as well as the openapi file
…lients Some of the models we duplicated from ocm commercial into aro-hcp are not currently used in the aro-hcp offering. We remove the `resource` definition for them. This ensures that the OCM SDK Go will not generate Go code to interact with the endpoints associated to these resources. The class and struct associated models will be kept so the Go data types themselves will still be generated. We do the latter because aro-hcp models that are currently in use might reference those. We performed this by copying the list of files containing _resource in their name from the ocm commercial models into aro-hcp models, excluding the resources files that already existed in aro-hcp models before the split/duplication occurred.
|
I will open another MR showcasing how this impacts the OCM SDK Go before merging this |
|
I opened a test MR that showcases the set of changes that will be applied on the OCM SDK Go side: openshift-online/ocm-sdk-go#1090 |
|
You can also review the commits individually to see how the conceptual changes were applied |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
As discussed in thread above, it'll be good to followup on cleaning up the model;
- for aro-hcp
- for commercial
so that the models become lean for both offerings.
Yes. But we should only consider start doing that once the full split is completed and only after some prudential time after that. In the meantime all offerings should still keep compatibility and consider the impact on other offerings. This is necessary so we can "roll back" in case something goes wrong during the process or during some prudential time after. |
@miguelsorianod Is this step necessary? Don't these models belong to resources which are not relevant [at the moment] to ARO-HCP? [e.g. add-on, etc..] and thus you indeed removed those in the 4th commit? I appreciate we might at some point in the future need a subset of these models [e.g. Upgrade Policy] but even than its worth revisiting only those specific model. |
No. In the 4th commit I only removed specification around This step is necessary. The reasons are several:
This focuses on achieving the split in the shortest amount of time possible getting the maximum possible autonomy. Once the split is fully completed we can then start cleaning up unneeded resources. However, not without waiting a prudential time as mentioned above. |
Gothca. This makes sense. Thank you. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: machi1990, miguelsorianod, nimrodshn The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM |
This MR performs a split/duplicate of the ARO-HCP API models.
By having a split/duplicate of the API models for ARO-HCP we are effectively separating the development of the API models between ARO-HCP and the other offerings. This means that changes in the aro-hcp models do not have an impact on the non aro-hcp ones and the other way around. This will give us increased autonomy at the API definition level at the expense of stopping to have re-usability.
At the point this MR is merged we have effectively started the split process between ARO-HCP and the other offerings. This is just one of the several steps and work that needs to be completed to have the split. The other offerings should continue to assume and work considering impacts on ARO-HCP until the whole split process is completed and a prudential time given.
Although we are splitting/duplicating the API models for ARO-HCP, no forking nor branching will happen on the ocm-api-model, ocm-api-metamodel , ocm-sdk-go, ocm-common, ocm-service-common repositories. This means that the offerings will still have shared dependencies in those repositories, even after the split. For example, ocm-sdk-go interfaces like the logging interface, some factories, etc… will still be shared. Teams across the offerings should continue to consider impact on other offerings around those areas.
Although we are splitting the models here, there are some exceptions where we will still have a few API level dependencies for some models. If interested in the details on those exceptions look at the
Decision on forking Clusters Service in ARO HCPdocument. Until we migrate those exceptions to our own ARO-HCP base endpoints, changes there can cause an impact cross-offering too.It should be safe to merge this for OCM Commercial, as this only modified aro-hcp data types.
For ARO-HCP, merging this will have an impact on the ARO-HCP RP: They will need to change the package of some data types to point to aro-hcp. This should be visible when they update the version of ocm-api-model and/or ocm-sdk-go. We evaluated the amount of changes and it is fairly small.
When updating the SDK in the CS repository we will need to adapt some ARO-HCP integration tests. The amount of changes is small.
Details
The MR changes can be explained in different conceptual steps:
remove referencing in models/aro_hcp
We achieve that by removing any referencing via the
ref statement in the models/aro_hcp definitions. We then
copy all the attributes for the resources we have defined
in the existing aro-hcp resources from model/clusters_mgmt,
even if not currently used by ARO-HCP.
However, we remove any locator statements to avoid providing
additional endpoints under the aro_hcp/v1alpha1 API URL for
the existing resources defined in aro-hcp.
The copying of all elements is needed to resolve all the attribute
references now that we removed the referencing mechanism. It also
ensures that there are no dependencies with ocm commercial from within
the aro-hcp models.
In this first step we still do not copy resources might be referenced by
the copied attributes.
This will end up adding many data types in the generated Go
models that won't be used and they will be available as data types.
This will also modify existing methods of the generated Go types
to use the duplicated data types. This can have an impact
on the consumers of the models but for aro-hcp case we expect
if they need to change it should not be a big amount of work.
copy models that exist in clusters_mgmt but not in aro_hcp models
This will ensure all the data types from ocm commercial are now
duplicated in aro-hcp. This means that the generated Go model will
now include all the new types.
regenerate models and openapi for aro-hcp
Now that we have duplicated all existing ocm commercial
models that did not exist in aro-hcp models we regenerate
the clientapi go data types as well as the openapi file
remove resource types for aro-hcp models we do not want to generate clients
Some of the models we duplicated from ocm commercial into
aro-hcp are not currently used in the aro-hcp offering. We
remove the
resourcedefinition for them. This ensuresthat the OCM SDK Go will not generate Go code to interact
with the endpoints associated to these resources.
The class and struct associated models will be kept so
the Go data types themselves will still be generated. We do
the latter because aro-hcp models that are currently in use
might reference those.
We performed this by copying the list of files containing
_resource in their name from the ocm commercial models
into aro-hcp models, excluding the resources files that
already existed in aro-hcp models before the split/duplication
occurred.