✨Fetch latest provider version if it's not set in the spec#180
Conversation
| var err error | ||
|
|
||
| // Create repository either from provided URL or from a config map. | ||
| if p.provider.GetSpec().FetchConfig != nil && p.provider.GetSpec().FetchConfig.Selector != nil { |
There was a problem hiding this comment.
can we optimize it and initialize the repo only once? It gets created in the later phases too
There was a problem hiding this comment.
yeah, I slightly rewrote the original PR, so it is more optimized now.
|
/hold @alexander-demicev agree, we can optimise this code a little. I'll propose another update for this PR soon |
|
/hold cancel |
2faa5a1 to
d81e26f
Compare
|
/retest |
04a3cf9 to
57264d7
Compare
|
/retest |
|
/retest |
| spec.Version = repo.DefaultVersion() | ||
|
|
||
| // Add version to the provider spec. | ||
| p.provider.SetSpec(spec) |
There was a problem hiding this comment.
why provider spec is set twice?
There was a problem hiding this comment.
The reason spec is set twice is because it accommodates two distinct workflows:
- When downloading manifests from GitHub/GitLab.
- When working with config maps containing manifests.
For the first workflow, the version is set during the manifest download process when a fetch URL is used. In the case of config maps, the version setting occurs in the load phase. Regardless of the workflow, the spec update is executed only once.
furkatgofurov7
left a comment
There was a problem hiding this comment.
What happens in case, CAPI tags v1.5.x as latest while CAPZ did not yet uplift to latest of CAPI and has latest tagged version compatible only with CAPI v1.4.x?
|
|
||
| configMaps := []corev1.ConfigMap{} | ||
|
|
||
| for _, fileName := range []string{"core-cluster-api-v1.4.2.yaml", "core-cluster-api-v1.4.3.yaml"} { |
There was a problem hiding this comment.
perhaps this the only way to do it in air gapped env (manually updating/adding huge manifests as CAPI releasing new versions), but I do not have a better suggestion either how we could improve it..
There was a problem hiding this comment.
yes, this is the only way. For clusterctl you have to create local repositories, for operator you can download manifests from github and make a configmap from file.
|
@furkatgofurov7 I guess the issue here is similar to when using |
That is true and I guess it is fine to follow the same behaviour here |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-demicev The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
LGTM label has been added. DetailsGit tree hash: 63a6154b1aab26d415d72dfa1308d997718ed432 |
|
/hold |
|
/hold cancel |
What this PR does / why we need it:
Now users have to manually set provider version, which is not convenient as it requires additional github searching. This PR allows to omit it, and the latest version will be substituted automatically. This behavior is similar to what is done in clusterctl, where version is optional.
Additionally this PR adds e2e tests for air-gapped environments.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #