🏃clusterctl: cleanup inventory #2277
Conversation
k8s-ci-robot
added
cncf-cla: yes
| // ClusterAPIVersion indicates the cluster API supported version. | ||
| ClusterAPIVersion string `json:"clusterAPIVersion,omitempty"` | ||
| // Contract defines the API Version of Cluster API (contract) supported by the ReleaseSeries. | ||
| Contract string `json:"contract,omitempty"` |
There was a problem hiding this comment.
Renaming ClusterAPIVersion into Contract so we are consistent with the term used in #2267 / less confusing
There was a problem hiding this comment.
Just out of my own curiosity, when you reference Cluster API (contract) are you referring to the clusterctl provider contract?
There was a problem hiding this comment.
I'm assuming there will be some documentation explaining Cluster API (contract) in the clusterctl upgrade section of the book.
There was a problem hiding this comment.
The API Version of Cluster API (contract) is v1alpha2, v1alpha3 etc. If this is not clear enough in the doc we should iterate and make it crystal clear
| // HasReleaseSeriesForVersion return true if the given version is in one of the ReleaseSeries. | ||
| func (m *Metadata) HasReleaseSeriesForVersion(version *version.Version) bool { | ||
| // GetReleaseSeriesForVersion return the release series for a given version. | ||
| func (m *Metadata) GetReleaseSeriesForVersion(version *version.Version) *ReleaseSeries { |
There was a problem hiding this comment.
Switching from Has to Get so I can reuse the same function in upgrade apply
| // InstanceName return the instance name for the provider, that is composed by the provider name and the namespace | ||
| // where the provider is installed (nb. clusterctl does not support more instance of the same provider to be | ||
| // installed in the same namespace) | ||
| func (p *Provider) InstanceName() string { |
There was a problem hiding this comment.
Adding instanceName so I can drop the usage of the name/namespace tuple in many places
|
|
||
| // List returns the inventory items for all the provider instances installed in the cluster. | ||
| List() ([]clusterctlv1.Provider, error) | ||
| List() (*clusterctlv1.ProviderList, error) |
There was a problem hiding this comment.
Switching from []clusterctlv1.Provider to clusterctlv1.ProviderList so I can drop the double List / list function
| } | ||
|
|
||
| // deriveManagementGroups derives the management groups from a list of providers. | ||
| func deriveManagementGroups(providerList *clusterctlv1.ProviderList) (ManagementGroupList, error) { |
There was a problem hiding this comment.
splitting deriveManagementGroups from reading providers so I can use to validate providers contracts during init
| // ManagementGroup is a group of providers composed by a CoreProvider and a set of Bootstrap/ControlPlane/Infrastructure providers | ||
| // watching objects in the same namespace. For example, a management group can be used for upgrades, in order to ensure all the providers | ||
| // in a management group support the same ClusterAPI version. | ||
| type ManagementGroup struct { |
There was a problem hiding this comment.
Adding some Utils to ManagementGroup/ManagementGroupList so It will be easier to leverage on these objects during upgrade apply
|
/area clusterctl |
vincepri
left a comment
vincepri
left a comment
There was a problem hiding this comment.
/approve
/assign @wfernandes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini, vincepri The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
fabriziopandini
force-pushed
the
clusterctl-cleanup-inventory
branch
from
18cd6ae to
8aa5b6a
Compare
|
@wfernandes comments addressed! |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
4 participants
What this PR does / why we need it:
This PR cleanup the inventory types and set the stages for implementing:
I have added comments explaining the main changes
Which issue(s) this PR fixes:
rif #1729