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.

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

Unify binary, repo and image naming #39

Closed
tnozicka opened this issue Dec 5, 2023 · 4 comments · Fixed by #56
Closed

Unify binary, repo and image naming #39

tnozicka opened this issue Dec 5, 2023 · 4 comments · Fixed by #56
Assignees
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@tnozicka
Copy link
Member

tnozicka commented Dec 5, 2023

We should unify the packages, binary and image name for consistency.

At this point the difference is confusing (caused by historical reasons when requesting the repo name):
repo name: github.com/scylladb/k8s-local-volume-provisioner
image name: docker.io/scylladb/k8s-local-volume-provisioner
binary name: local-csi-driver

@scylla-operator-bot scylla-operator-bot bot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Dec 5, 2023
@tnozicka tnozicka added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Dec 5, 2023
@scylla-operator-bot scylla-operator-bot bot removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Dec 5, 2023
@tnozicka tnozicka added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Dec 5, 2023
@tnozicka tnozicka removed the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Apr 4, 2024
Copy link
Contributor

The Scylla Operator project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 30d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out

/lifecycle stale

@scylla-operator-bot scylla-operator-bot bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 7, 2024
@tnozicka
Copy link
Member Author

tnozicka commented Jul 8, 2024

/remove-lifecycle stale
/triage accepted

@scylla-operator-bot scylla-operator-bot bot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 8, 2024
@zimnx
Copy link
Collaborator

zimnx commented Jul 10, 2024

I collected pros and cons behind few options we discussed, here's a summary:


Change k8s-local-volume-provisioner to local-csi-driver

  • (pro) name convention matches other drivers available
  • (pro) simple user update, it requires changing an image used in manifests. Namespace and object names stay the same.

What needs to be done afterwards:

  • rename the Github repo
  • create new quay/docker repo, gain access for CI jobs through IT/Tomas
  • update all jobs in CI to use new name

Change local-csi-driver to k8s-local-volume-provisioner

  • (pro) name convention matches kubernetes-sigs local-static-volume-provisioner
  • (con) conterargument about above, this project doesn't provide any provisioner implementation, it's being used as a dependency
  • (con) k8s prefix might be an issue, as drivers are designed to be run not only on kubernetes - although there are no plans for having support for other orchestrators
  • (con) update requires manually deleting namespace first and only then applying new manifests

What needs to be done afterwards:

  • rename binary and names in manifests in CI, Operator and repo itself

Change k8s-local-volume-provisioner to scylladb-local-csi(-driver)

  • (pro) scylla prefix prevents collision with other solutions
  • (con) nothing is related to scylla, we are just the maintainers
  • (con) scylla prefix may be confusing, users can use GCP, AWS CSI or other drivers and run scylla without known issues

What needs to be done afterwards:

  • rename Github repo
  • create new quay/docker repo, gain access for CI jobs through IT/Tomas
  • update all jobs in CI to use new name
  • rename binary and names in manifests in CI, Operator and repo itself

I had a meeting with @ylebi where we discussed them, and we are align on opting to the first option.
@tnozicka what do you think?

@tnozicka
Copy link
Member Author

Thanks for writing this down.

I'd generally be in favour of using scylla- prefix for our namespace

(con) nothing is related to scylla, we are just the maintainers

Kind of, but the name is generic enough for other people writing a CSI driver to pick the same name and conflict with us. Having such prefix is pretty common practice to avoid collisions on naming.
Both our GH repo and the container registry already have the prefix in the organization part, so this is meant only for the namespace.

That said, going for the first option Change k8s-local-volume-provisioner to local-csi-driver is fine with me, the non-prefixed namespace is already there, this won't make it worse and it can be done separately later, if we decide to go there.

But please start by renaming the repo, so the opinionated part (which is how this discrepancy came to be) will be sorted out.

ylebi added a commit to ylebi/k8s-local-volume-provisioner that referenced this issue Jul 11, 2024
Rename k8s-local-volume-provisioner to local-csi-driver
ylebi added a commit to ylebi/k8s-local-volume-provisioner that referenced this issue Jul 16, 2024
Rename k8s-local-volume-provisioner to local-csi-driver

Update pkg/cmd/local-csi-driver/driver.go

Co-authored-by: Tomáš Nožička <tnozicka@users.noreply.github.com>

Update pkg/cmd/local-csi-driver/driver.go

Update pkg/cmd/local-csi-driver/driver.go

Update test/e2e/set/localdriver/upstream.go

Update test/e2e/set/localdriver/upstream.go

Co-authored-by: Tomáš Nožička <tnozicka@users.noreply.github.com>

Update test/e2e/set/localdriver/upstream.go

Update README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants