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

core: change directory layout #8740

Merged
merged 1 commit into from
Nov 30, 2021
Merged

core: change directory layout #8740

merged 1 commit into from
Nov 30, 2021

Conversation

leseb
Copy link
Member

@leseb leseb commented Sep 17, 2021

Description of your changes:

As per discussion, proposing a new layout for the charts/yaml/olm files.

./deploy
├── charts
│   ├── rook-ceph
│   │   └── templates
│   └── rook-ceph-cluster
│       └── templates
├── examples
│   ├── csi
│   │   ├── cephfs
│   │   └── rbd
│   ├── flex
│   ├── monitoring
│   ├── pre-k8s-1.16
└── olm
    └── assemble

Signed-off-by: Sébastien Han seb@redhat.com

Which issue is resolved by this Pull Request:
Resolves #

Checklist:

  • Commit Message Formatting: Commit titles and messages follow guidelines in the developer guide.
  • Skip Tests for Docs: Add the flag for skipping the build if this is only a documentation change. See here for the flag.
  • Skip Unrelated Tests: Add a flag to run tests for a specific storage provider. See test options.
  • Reviewed the developer guide on Submitting a Pull Request
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.
  • Pending release notes updated with breaking and/or notable changes, if necessary.
  • Upgrade from previous release is tested and upgrade user guide is updated, if necessary.
  • Code generation (make codegen) has been run to update object specifications, if necessary.

@leseb leseb added this to In progress in v1.8 via automation Sep 17, 2021
@mergify
Copy link

mergify bot commented Sep 17, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/master/development-flow.html#updating-your-fork

Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

Why change cluster to deploy? IMO cluster works well already for the root folder, and examples is easier to understand than manual.

./cluster
├── charts
│   ├── rook-ceph
│   │   └── templates
│   └── rook-ceph-cluster
│       └── templates
├── examples
│   ├── csi
│   │   ├── cephfs
│   │   └── rbd
│   ├── flex
│   ├── monitoring
│   ├── pre-k8s-1.16
│   └── test-data
└── olm
    └── assemble

Documentation/ceph-common-issues.md Outdated Show resolved Hide resolved
v1.8 automation moved this from In progress to Review in progress Sep 17, 2021
@leseb
Copy link
Member Author

leseb commented Sep 17, 2021

Why change cluster to deploy? IMO cluster works well already for the root folder, and examples is easier to understand than manual.

./cluster
├── charts
│   ├── rook-ceph
│   │   └── templates
│   └── rook-ceph-cluster
│       └── templates
├── examples
│   ├── csi
│   │   ├── cephfs
│   │   └── rbd
│   ├── flex
│   ├── monitoring
│   ├── pre-k8s-1.16
│   └── test-data
└── olm
    └── assemble

Honestly, I've always struggled to understand why we had cluster. IMO deploy works better, if I'm the root of the repository and I'm looking for deployment files, then deploy makes sense.
Following this, you can choose between a manual deployment, chartsdeployment with helm or olm for a deployment with OLM. If we find a better name than manual I'm open for changes.

Let's discuss this in the huddle :)

@BlaineEXE
Copy link
Member

A tangential question: why is test-data/ in the examples dir? Shouldn't this be under our tests dir?

@leseb
Copy link
Member Author

leseb commented Sep 20, 2021

A tangential question: why is test-data/ in the examples dir? Shouldn't this be under our tests dir?

Good point, let me move it out.

@leseb leseb force-pushed the examples-layout branch 3 times, most recently from ede2b37 to 00ac445 Compare September 20, 2021 10:23
@leseb leseb requested a review from travisn September 20, 2021 10:28
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

The directory changes look good, pending discussion in the community meeting.

With all the whitespace and formatting changes, it's going to cause a lot of merge conflicts with backports since these changes will just stay in master. What if we had a separate PR for all the whitespace and formatting that we do backport to 1.7 and keep this change just for the directory rename?

@mergify
Copy link

mergify bot commented Sep 20, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/master/development-flow.html#updating-your-fork

@mergify
Copy link

mergify bot commented Sep 21, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/master/development-flow.html#updating-your-fork

@leseb leseb force-pushed the examples-layout branch 3 times, most recently from f5475ba to 272d577 Compare September 21, 2021 17:11
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

Is it too early in the 1.8 cycle for this with all the potential backport conflicts? Or maybe we should just go ahead with all the big changes and plan on resolving more backport conflicts...

Documentation/ceph-filesystem-crd.md Outdated Show resolved Hide resolved
deploy/charts/rook-ceph/values.yaml Outdated Show resolved Hide resolved
tests/framework/installer/settings.go Outdated Show resolved Hide resolved
.github/workflows/canary-integration-test-arm64.yml Outdated Show resolved Hide resolved
@mergify
Copy link

mergify bot commented Sep 21, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/master/development-flow.html#updating-your-fork

@leseb
Copy link
Member Author

leseb commented Sep 22, 2021

Is it too early in the 1.8 cycle for this with all the potential backport conflicts? Or maybe we should just go ahead with all the big changes and plan on resolving more backport conflicts...

It is going to introduce more conflicts. I don't mind holding this one for a month or so if it's for everyone's best.

@mergify
Copy link

mergify bot commented Sep 23, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/latest/development-flow.html#updating-your-fork

@mergify
Copy link

mergify bot commented Sep 24, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/latest/development-flow.html#updating-your-fork

@mergify
Copy link

mergify bot commented Sep 27, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/latest/development-flow.html#updating-your-fork

@leseb
Copy link
Member Author

leseb commented Sep 28, 2021

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Sep 28, 2021

Command rebase: failure

Pull request can't be updated with latest base branch changes
GitHub App like Mergify are not allowed to rebase pull request where .github/workflows is changed.
This pull request must be rebased manually.
err-code: EEA95

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale Labeled by the stale bot label Oct 28, 2021
@BlaineEXE BlaineEXE removed the stale Labeled by the stale bot label Nov 3, 2021
@BlaineEXE
Copy link
Member

Not stale. Just waiting until we start releasing v1.8 betas.

@mergify
Copy link

mergify bot commented Nov 23, 2021

This pull request has merge conflicts that must be resolved before it can be merged. @leseb please rebase it. https://rook.io/docs/rook/latest/development-flow.html#updating-your-fork

@leseb leseb force-pushed the examples-layout branch 7 times, most recently from a0217c5 to eb594cc Compare November 29, 2021 15:44
Copy link
Member

@travisn travisn left a comment

Choose a reason for hiding this comment

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

Approving, with a few minor issues that would be best to fix before merge if possible

Documentation/ceph-block.md Outdated Show resolved Hide resolved
deploy/charts/rook-ceph/values.yaml Outdated Show resolved Hide resolved
deploy/charts/rook-ceph/values.yaml Outdated Show resolved Hide resolved
As per discussion, proposing a new layout for the charts/yaml/olm files.

./deploy
├── charts
│   ├── rook-ceph
│   │   └── templates
│   └── rook-ceph-cluster
│       └── templates
├── examples
│   ├── csi
│   │   ├── cephfs
│   │   └── rbd
│   ├── flex
│   ├── monitoring
│   ├── pre-k8s-1.16
└── olm
    └── assemble

Signed-off-by: Sébastien Han <seb@redhat.com>
@leseb leseb merged commit decc481 into rook:master Nov 30, 2021
v1.8 automation moved this from Review in progress to Done Nov 30, 2021
@leseb leseb deleted the examples-layout branch November 30, 2021 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v1.8
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants