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.

Sign up for GitHub

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

Jump to bottom

Kopia RepositoryServer CRD in Kanister #1645

Merged
merged 68 commits into from
Mar 1, 2023
Merged

Kopia RepositoryServer CRD in Kanister #1645

merged 68 commits into from
Mar 1, 2023

Conversation

shlokc9
Copy link
Contributor

@shlokc9 shlokc9 commented Sep 22, 2022 •
edited

Change Overview

This PR introduces the RepositoryServer CRD in Kanister
It has all the skeleton code generated by the using code-generator library (one of the controller-tools in Kubebuilder)

Pull request type

Please check the type of change your PR introduces:

  • 🚧 Work in Progress
  • 🌈 Refactoring (no functional changes, no api changes)
  • 🐹 Trivial/Minor
  • 🐛 Bugfix
  • 🌻 Feature
  • 🗺️ Documentation
  • 🤖 Test

Issues

  • fixes #issue-number

Test Plan

  • 💪 Manual
  • ⚡ Unit test
  • 💚 E2E

@github-actions
Copy link
Contributor

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

@infraq infraq added this to In Progress in Kanister Sep 22, 2022
ihcsim
ihcsim reviewed Sep 22, 2022
View reviewed changes
pkg/kopia/repositoryserver/README.md Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/config/cr.kanister.io_repositoryservers.yaml Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/main.go Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/manifests/cr.kanister.io_repositoryservers.yaml Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/pkg/controller/repositoryserver.go Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/pkg/controller/repositoryserver.go Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/main.go Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/pkg/controller/repositoryserver.go Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/pkg/controller/repositoryserver.go Outdated Show resolved Hide resolved
pkg/kopia/repositoryserver/pkg/controller/repositoryserver.go Outdated Show resolved Hide resolved
@shlokc9 shlokc9 mentioned this pull request Sep 27, 2022
Merged
10 tasks
@shlokc9
Copy link
Contributor Author

shlokc9 commented Sep 28, 2022 •
edited

@ihcsim @viveksinghggits I was mentioning this to @pavannd1 as a floating thought. Based on the current work for RepositoryServer controller;

Will it be a good idea to create a common interface for all controllers in Kanister, sometime in future?

  • Objective is to abstract all the reusable code needed to set up a controller in Kanister
  • This would make it easy to create a controller for any new CRDs that might come up in Kanister
  • Users could then just create the CRD, register it with a concrete controller and then define add/update/delete resourceHandlers for the CRD
  • I think we have all the methods already present in Kanister that can be used to define an abstract controller
  • This would also include refactoring the Kanister controller and RepositoryServer controller to inherit the abstract interface

Let me know your thoughts... :)

@shlokc9 shlokc9 changed the title Add RepositoryServer CR and Controller Add RepositoryServer CRD Sep 30, 2022
ankitjain235
ankitjain235 reviewed Oct 3, 2022
View reviewed changes
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

Added some suggestions

pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
pkg/apis/cr/v1alpha1/types.go Outdated Show resolved Hide resolved
@shlokc9 shlokc9 marked this pull request as ready for review October 7, 2022 09:19
@shlokc9 shlokc9 force-pushed the kopia-repository-server-crd branch 2 times, most recently from 5e77e9b to cbb3d82 Compare October 26, 2022 14:13
ankitjain235
ankitjain235 reviewed Nov 16, 2022
View reviewed changes
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

Added some comments. Should we some details in CRD field's descriptions, so that it has relevant information?

docker/repositoryserver-controller/Dockerfile Outdated Show resolved Hide resolved
docker/repositoryserver-controller/Dockerfile Outdated Show resolved Hide resolved
pkg/customresource/repositoryserver.yaml Outdated Show resolved Hide resolved
@kale-amruta kale-amruta removed the request for review from ihcsim December 19, 2022 08:14
@kale-amruta kale-amruta self-assigned this Dec 21, 2022
shlokc9 and others added 5 commits January 18, 2023 13:33
@shlokc9 @kale-amruta
Added the manifests for CRD and example CR along with a basic CR cont…
3d124df 
…roller to log outputs
@ihcsim @kale-amruta
Run the triage workflow on primary repo only (#1642)
57c850e 
Signed-off-by: Ivan Sim <ivan.sim@kasten.io>
@dependabot @kale-amruta
Bump github.com/vmware/govmomi from 0.28.0 to 0.29.0 (#1599)
cf64429 
Bumps [github.com/vmware/govmomi](https://github.com/vmware/govmomi) from 0.28.0 to 0.29.0.
- [Release notes](https://github.com/vmware/govmomi/releases)
- [Changelog](https://github.com/vmware/govmomi/blob/master/CHANGELOG.md)
- [Commits](vmware/govmomi@v0.28.0...v0.29.0)

---
updated-dependencies:
- dependency-name: github.com/vmware/govmomi
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@shlokc9 @kale-amruta
Update the command paths in pkg/kopia/repositoryserver/README.md
51ac921
@shlokc9 @kale-amruta
Update the README.md
8c40f40
@kale-amruta
Copy link
Contributor

@PrasadG193 I have moved the makefile changes to #1871 and I am addressing the comments there, since all the goreleaser related changes are there in that PR

@PrasadG193
Copy link
Contributor

Sounds good @kale-amruta. Can we merge this PR independently?

@viveksinghggits
Copy link
Contributor

If it's dependent on the PR #1871. We should change the base branch to make sure we are not seeing the changes that are already reviewed as part of other PR.

@kale-amruta
Copy link
Contributor

@PrasadG193 yes this PR can be merged independently. @viveksinghggits this is the base branch for all the other branches, this is not dependent on any other branch

@viveksinghggits viveksinghggits changed the title Add RepositoryServer CRD Implement Kopia RepositoryServer in Kanister Feb 17, 2023
PrasadG193
PrasadG193 approved these changes Feb 21, 2023
View reviewed changes
Copy link
Contributor

@PrasadG193 PrasadG193 left a comment

Choose a reason for hiding this comment

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

LGTM

Kanister automation moved this from Review Required to Reviewer approved Feb 21, 2023
ankitjain235
ankitjain235 approved these changes Feb 22, 2023
View reviewed changes
Copy link
Contributor

@ankitjain235 ankitjain235 left a comment

Choose a reason for hiding this comment

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

LGTM.

pavannd1
pavannd1 reviewed Feb 22, 2023
View reviewed changes
pkg/customresource/repositoryserver.yaml Outdated Show resolved Hide resolved
pkg/customresource/repositoryserver.yaml Show resolved Hide resolved
pkg/customresource/repositoryserver.yaml Show resolved Hide resolved
pkg/customresource/repositoryserver.yaml Show resolved Hide resolved
pkg/customresource/repositoryserver.yaml Outdated Show resolved Hide resolved
@pavannd1 pavannd1 changed the title Implement Kopia RepositoryServer in Kanister Kopia RepositoryServer CRD in Kanister Feb 22, 2023
@mergify mergify bot merged commit afe5fb3 into master Mar 1, 2023
Kanister automation moved this from Reviewer approved to Done Mar 1, 2023
@mergify mergify bot deleted the kopia-repository-server-crd branch March 1, 2023 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ihcsim ihcsim ihcsim left review comments

@kale-amruta kale-amruta kale-amruta left review comments

@pavannd1 pavannd1 pavannd1 approved these changes

@PrasadG193 PrasadG193 PrasadG193 approved these changes

@ankitjain235 ankitjain235 ankitjain235 approved these changes

@viveksinghggits viveksinghggits Awaiting requested review from viveksinghggits

Assignees

@kale-amruta kale-amruta

Labels
kueue
Projects
Kanister
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@shlokc9 @PrasadG193 @kale-amruta @viveksinghggits @ihcsim @pavannd1 @ankitjain235