⚠️ clusterctlv2: add file system backend implementations for provider's repository #1963

Arvinderpal · 2019-12-24T19:31:42Z

What this PR does / why we need it:
This PR adds a file system backend implementations for provider's repository.

Which issue(s) this PR fixes
Rif #1729

/assign @fabriziopandini
/cc @detiber @ncdc @vincepri @jiatongw

k8s-ci-robot · 2019-12-24T19:31:50Z

Welcome @Arvinderpal!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2019-12-24T19:31:51Z

Hi @Arvinderpal. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

fabriziopandini · 2019-12-28T12:58:50Z

@Arvinderpal thanks for this PR!
FYI the work on new backends is on hold on the clusterctl tracking issue because most probably there will be some rework in the Repository interface, due to new requirements under definition for supporting upgrades.
I will keep you posted
/hold

fabriziopandini · 2019-12-31T11:51:14Z

@Arvinderpal, as anticipated #1978 implements the change of the Repository interface.
hopefully, this should not be a big change for your PR (everything you need is already implemented in getLatestRelease, even if some refactors are required).

vincepri · 2020-01-03T16:13:20Z

/ok-to-test

ncdc · 2020-01-16T21:52:35Z

@fabriziopandini is this needed for v0.3.0? I'm trying to wrangle PRs-vs-milestones and want to make sure I put it in the right place. Thanks!

Arvinderpal · 2020-01-16T22:03:24Z

This needs to incorporate the recent change to the Repository interface, which I think is pretty straightforward and I can do that asap. However, the PR still needs a code review.

fabriziopandini

@Arvinderpal
The PR is definitely on the right track, pending the change on the interface.
It would be also great if we can simplify the URL syntax and make it more flexible and at the same time more consistent with other clusterctl features.
I also added some nits on test messages for consistency with the rest of the codebase

@ncdc It would be great to get this into the milestone so we can provide a more articulated solution for the air-gapped scenario

cmd/clusterctl/pkg/client/repository/repository_local_unix.go

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go

Arvinderpal

@fabriziopandini Thanks for the feedback. I have incorporated all the suggested changes. Please do another pass.

cmd/clusterctl/pkg/client/repository/repository_local_unix.go

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go

fabriziopandini

@Arvinderpal thanks!

cmd/clusterctl/pkg/client/repository/repository_local_unix.go

Arvinderpal

@fabriziopandini Addressed all your comments. Please have another look. Thanks.

cmd/clusterctl/pkg/client/repository/repository_local_unix.go

fabriziopandini

@Arvinderpal thanks
One last mini nit from my side + some CI checks error to be fixed
/hold cancel

@ncdc @vincepri this PR add an important piece for the air-gapped support in clusterctl. PTAL

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go

Arvinderpal · 2020-01-24T16:37:49Z

BTW, pull-cluster-api-test is constantly failing. Appears to be a controller-runtime issue. I have filed a fix for that here: kubernetes-sigs/controller-runtime#781

ncdc · 2020-01-24T17:22:34Z

BTW, pull-cluster-api-test is constantly failing. Appears to be a controller-runtime issue.

The master branch doesn't have this problem (consistent failures). Please try rebasing to see if that fixes your issue. I think your controller-runtime PR is 👍, but our tests should be passing, and if they're consistently failing with this PR, then it's likely localized to your branch.

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go

vincepri · 2020-01-24T17:26:43Z

FYI the commits shouldn't include references to issues or usernames

ncdc · 2020-01-24T17:27:12Z

The test failures were because you were deleting /tmp and not the tempdir you just created. My latest review comments should resolve the issues.

Arvinderpal · 2020-01-25T02:17:21Z

The test failures were because you were deleting /tmp and not the tempdir you just created. My latest review comments should resolve the issues.

@ncdc Hmm...I really don't see where I'm deleting all of /tmp. Can you tell me exactly where you saw this? I see that /tmp is just fine when I run my tests locally and only the tmpDir is removed.
Besides that I'll investigate some more as to why my branch would fail on that particular test while master is passing. I have rebased several times with master, so ¯_(ツ)_/¯

ncdc · 2020-01-25T12:17:21Z

You were deleting filepath.Dir(tmpDir). That resolves to /tmp

Arvinderpal · 2020-01-25T15:43:11Z

Squashed all the commits into one and removed username reference from message. If there are no final concerns, please go ahead and merge this.

cmd/clusterctl/pkg/client/repository/repository_local_unix.go

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go

ncdc · 2020-01-27T15:29:03Z

/approve

@fabriziopandini over to you for lgtm when you're ready

fabriziopandini

@Arvinderpal thanks!
/lgtm
/approve

k8s-ci-robot · 2020-01-28T20:12:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Arvinderpal, fabriziopandini, ncdc

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:

~~OWNERS~~ [ncdc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot assigned fabriziopandini Dec 24, 2019

k8s-ci-robot requested review from detiber, jiatongw, ncdc and vincepri December 24, 2019 19:31

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 24, 2019

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 24, 2019

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Dec 24, 2019

Arvinderpal mentioned this pull request Dec 26, 2019

Clusterctl v2: support for additional backend implementations for provider repository #1964

Closed

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 28, 2019

fabriziopandini mentioned this pull request Dec 31, 2019

Tracking issue for clusterctl v2 implementation #1729

Closed

62 tasks

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 3, 2020

fabriziopandini requested changes Jan 17, 2020

View reviewed changes

ncdc added this to the v0.3.0 milestone Jan 17, 2020

Arvinderpal commented Jan 19, 2020

View reviewed changes

Arvinderpal force-pushed the clusterctlv2-repository-local branch 3 times, most recently from 39b7f93 to ae46ed3 Compare January 21, 2020 13:55

fabriziopandini requested changes Jan 21, 2020

View reviewed changes

Arvinderpal commented Jan 21, 2020

View reviewed changes

fabriziopandini reviewed Jan 22, 2020

View reviewed changes

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go Outdated Show resolved Hide resolved

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 22, 2020

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jan 24, 2020

Arvinderpal force-pushed the clusterctlv2-repository-local branch from 5bcd109 to 71ed84b Compare January 24, 2020 14:20

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jan 24, 2020

ncdc reviewed Jan 24, 2020

View reviewed changes

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go Outdated Show resolved Hide resolved

ncdc reviewed Jan 24, 2020

View reviewed changes

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go Outdated Show resolved Hide resolved

ncdc reviewed Jan 24, 2020

View reviewed changes

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go Outdated Show resolved Hide resolved

aaroniscode mentioned this pull request Jan 24, 2020

Allow clusterctl to specify a custom Workload Cluster template #2133

Closed

Arvinderpal force-pushed the clusterctlv2-repository-local branch from 71ed84b to bd75f0f Compare January 25, 2020 02:12

Arvinderpal force-pushed the clusterctlv2-repository-local branch from bd75f0f to a114a97 Compare January 25, 2020 15:41

k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Jan 25, 2020

ncdc reviewed Jan 27, 2020

View reviewed changes

cmd/clusterctl/pkg/client/repository/repository_local_unix.go Outdated Show resolved Hide resolved

cmd/clusterctl/pkg/client/repository/repository_local_unix_test.go Outdated Show resolved Hide resolved

File system backend implementation for provider's repository.

b5b9f79

Arvinderpal force-pushed the clusterctlv2-repository-local branch from a114a97 to b5b9f79 Compare January 27, 2020 15:27

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 27, 2020

fabriziopandini approved these changes Jan 28, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 28, 2020

k8s-ci-robot merged commit 52117a9 into kubernetes-sigs:master Jan 28, 2020

fabriziopandini mentioned this pull request Jan 28, 2020

clusterctl repositories: don't exclude versions with semver build metadata #2157

Closed

Arvinderpal mentioned this pull request Jan 28, 2020

✨ clusterctl: Add automatic selection of repository backend based on URL #2192

Merged

alanmeadows mentioned this pull request Feb 10, 2020

airshipctl and clusterctl integration airshipit/airshipctl#29

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚠️ clusterctlv2: add file system backend implementations for provider's repository #1963

⚠️ clusterctlv2: add file system backend implementations for provider's repository #1963

Arvinderpal commented Dec 24, 2019

k8s-ci-robot commented Dec 24, 2019

k8s-ci-robot commented Dec 24, 2019

fabriziopandini commented Dec 28, 2019

fabriziopandini commented Dec 31, 2019

vincepri commented Jan 3, 2020

ncdc commented Jan 16, 2020

Arvinderpal commented Jan 16, 2020

fabriziopandini left a comment

Arvinderpal left a comment

fabriziopandini left a comment

Arvinderpal left a comment

fabriziopandini left a comment

Arvinderpal commented Jan 24, 2020

ncdc commented Jan 24, 2020

vincepri commented Jan 24, 2020

ncdc commented Jan 24, 2020

Arvinderpal commented Jan 25, 2020

ncdc commented Jan 25, 2020

Arvinderpal commented Jan 25, 2020

ncdc commented Jan 27, 2020

fabriziopandini left a comment

k8s-ci-robot commented Jan 28, 2020

⚠️ clusterctlv2: add file system backend implementations for provider's repository #1963

⚠️ clusterctlv2: add file system backend implementations for provider's repository #1963

Conversation

Arvinderpal commented Dec 24, 2019

k8s-ci-robot commented Dec 24, 2019

k8s-ci-robot commented Dec 24, 2019

fabriziopandini commented Dec 28, 2019

fabriziopandini commented Dec 31, 2019

vincepri commented Jan 3, 2020

ncdc commented Jan 16, 2020

Arvinderpal commented Jan 16, 2020

fabriziopandini left a comment

Choose a reason for hiding this comment

Arvinderpal left a comment

Choose a reason for hiding this comment

fabriziopandini left a comment

Choose a reason for hiding this comment

Arvinderpal left a comment

Choose a reason for hiding this comment

fabriziopandini left a comment

Choose a reason for hiding this comment

Arvinderpal commented Jan 24, 2020

ncdc commented Jan 24, 2020

vincepri commented Jan 24, 2020

ncdc commented Jan 24, 2020

Arvinderpal commented Jan 25, 2020

ncdc commented Jan 25, 2020

Arvinderpal commented Jan 25, 2020

ncdc commented Jan 27, 2020

fabriziopandini left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Jan 28, 2020