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

fix: pvc pending #885

Merged
merged 4 commits into from
Dec 19, 2023
Merged

fix: pvc pending #885

merged 4 commits into from
Dec 19, 2023

Conversation

jayl1e
Copy link
Contributor

@jayl1e jayl1e commented Dec 19, 2023

Why:

  • ceph-filesystem is replaced by nfs
  • move to jenkins-cd dir

Close: #655

Signed-off-by: lijie <lijie@pingcap.com>
@ti-chi-bot ti-chi-bot bot requested a review from purelind December 19, 2023 08:51
@ti-chi-bot ti-chi-bot bot added the area/apps label Dec 19, 2023
@ti-chi-bot ti-chi-bot bot requested a review from wuhuizuo December 19, 2023 08:51
@ti-chi-bot ti-chi-bot bot added the env/prod will deploy on the main product cluster label Dec 19, 2023
Copy link
Contributor

ti-chi-bot bot commented Dec 19, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.

Review for "fix: pvc pending"

Summary

This pull request replaces the storage class from ceph-filesystem to nfs and moves the pvc.yaml file from apps/prod/ccache-pvc/base to apps/prod/jenkins/post/cd.

Potential Problems

There are no major potential problems with this pull request. However, the description could be clearer as to why the storage class is being changed. Additionally, it might be useful to explain why pvc.yaml is being moved to a different directory.

Suggestions

  • Clarify the reason for changing the storage class from ceph-filesystem to nfs in the description.
  • Provide more context around why the pvc.yaml file is being moved to a different directory in the description.
  • Consider adding comments to the pvc.yaml file to explain the purpose of each PVC.

@ti-chi-bot ti-chi-bot bot added the size/XS label Dec 19, 2023
Signed-off-by: lijie <lijie@pingcap.com>
Copy link
Contributor

ti-chi-bot bot commented Dec 19, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.

Pull Request Review

Summary

This PR aims to replace ceph-filesystem with nfs and move to the jenkins-cd directory. It renames the ccache-pvc directory to pvc and updates the kustomization.yaml and pvc.yaml files in the pvc directory.

Changes Made

  • Replaced ceph-filesystem with nfs.
  • Moved to jenkins-cd directory.
  • Renamed ccache-pvc directory to pvc.
  • Updated kustomization.yaml and pvc.yaml files in the pvc directory.

Potential Problems

  • There may be other parts of the codebase that rely on ceph-filesystem that could be affected by this change. It would be important to ensure that all other areas of the codebase that use this storageClassName are updated accordingly.
  • It is unclear if there are any other changes or updates made to the codebase beyond what is shown in the diff. If there are additional changes made, it would be important to ensure they do not introduce any new bugs or issues.

Suggestions

  • Ensure that all other areas of the codebase that use ceph-filesystem are updated accordingly.
  • Test the updated codebase thoroughly to ensure there are no new bugs or issues introduced.
  • If there are any additional changes made to the codebase beyond what is shown in the diff, ensure they do not introduce any new bugs or issues.

Signed-off-by: lijie <lijie@pingcap.com>
Copy link
Contributor

ti-chi-bot bot commented Dec 19, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.

Review of "fix: pvc pending"

Summary

This pull request replaces the usage of ceph-filesystem with nfs and moves some files to a different directory.

Potential Problems

Based on the diff, there are no significant problems in this pull request.

Fixing Suggestions

No fixing suggestions are necessary as the changes look good.

Overall

This is a simple and straightforward pull request that makes sensible changes to the codebase. It should be merged as soon as possible.

Signed-off-by: lijie <lijie@pingcap.com>
Copy link
Contributor

ti-chi-bot bot commented Dec 19, 2023

I have already done a preliminary review for you, and I hope to help you do a better job.

Review of the GitHub pull request

Summary

This pull request deletes the pvc.yaml file and adds a new pvc.yaml file in the jenkins-cd directory. The new pvc.yaml file contains new PersistentVolumeClaims with a new storage class nfs. The changes have been made to improve the functionality of the system.

Potential Problems

There are no potential problems with the changes being made in this pull request.

Fixing Suggestions

There are no fixing suggestions required for this pull request. The changes made in this pull request are good and can be merged.

@ti-chi-bot ti-chi-bot bot added size/L and removed size/XS labels Dec 19, 2023
Copy link
Contributor

ti-chi-bot bot commented Dec 19, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wuhuizuo

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:

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

Copy link
Contributor

ti-chi-bot bot commented Dec 19, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-12-19 10:00:20.823467124 +0000 UTC m=+955111.860694051: ☑️ agreed by wuhuizuo.

@ti-chi-bot ti-chi-bot bot added the approved label Dec 19, 2023
@ti-chi-bot ti-chi-bot bot merged commit 6dfcc85 into main Dec 19, 2023
4 checks passed
@ti-chi-bot ti-chi-bot bot deleted the fix/pvc branch December 19, 2023 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/apps env/prod will deploy on the main product cluster lgtm size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move ccache-pvc to directory in jenkins-cd namespace
2 participants