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

Add optional field to generate log/data pvc #1321

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add optional field to generate log/data pvc #1321

wants to merge 2 commits into from

Conversation

drivebyer
Copy link
Contributor

@drivebyer drivebyer commented Jul 4, 2023 •
edited
Loading

All Submissions:

In some cases, we only need one pvc to store log and data.

  • Have you opened an Issue before filing this PR?
  • Have you signed our CLA?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if such).

@drivebyer
Copy link
Contributor Author

drivebyer commented Jul 4, 2023 •
edited
Loading

PTAL @irajdeep @slaskawi

@drivebyer drivebyer mentioned this pull request Aug 6, 2023
Closed
@rdxmb
Copy link

rdxmb commented Aug 15, 2023

Why logging into the filesystem? Logs in kubernetes should go to stdout/stderr.

@github-actions
Copy link
Contributor

This PR is being marked stale because it has been open for 60 days with no activity. Please update the PR or ask for a fresh review.

@github-actions github-actions bot added the stale label Oct 15, 2023
@irajdeep irajdeep added safe-to-test Add this label to PRs from forks to trigger E2E tests and removed stale labels Jan 17, 2024
@irajdeep
Copy link
Contributor

@drivebyer This looks reasonable to me. Would you mind rebasing this on top of master and resolving the merge conflict please?

@drivebyer
Copy link
Contributor Author

@drivebyer This looks reasonable to me. Would you mind rebasing this on top of master and resolving the merge conflict please?

of course

@drivebyer
Copy link
Contributor Author

@irajdeep It's been done

@irajdeep
Copy link
Contributor

Could you look into the failing unit tests.

@drivebyer
Copy link
Contributor Author

hi @irajdeep I change the filed name to CombineDataAndLogsVolumes, then the less code we need to change.

@github-actions GitHub Actions
Copy link
Contributor

This PR is being marked stale because it has been open for 60 days with no activity. Please update the PR or ask for a fresh review.

@github-actions github-actions bot added the stale label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@irajdeep irajdeep Awaiting requested review from irajdeep

@mircea-cosbuc mircea-cosbuc Awaiting requested review from mircea-cosbuc mircea-cosbuc is a code owner

@lsierant lsierant Awaiting requested review from lsierant lsierant is a code owner

@slaskawi slaskawi Awaiting requested review from slaskawi slaskawi is a code owner

@nammn nammn Awaiting requested review from nammn nammn is a code owner

@Julien-Ben Julien-Ben Awaiting requested review from Julien-Ben Julien-Ben is a code owner

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
safe-to-test Add this label to PRs from forks to trigger E2E tests stale
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@drivebyer @rdxmb @irajdeep