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

Set RepositoryServerCR Conditions #2169

Merged
merged 35 commits into from
Aug 9, 2023
Merged

Set RepositoryServerCR Conditions #2169

merged 35 commits into from
Aug 9, 2023

Conversation

akankshakumari393
Copy link
Contributor

@akankshakumari393 akankshakumari393 commented Jul 12, 2023 •
edited

Change Overview

This PR sets Repository Server CR Conditions based on different errors.

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

https://gist.github.com/akankshakumari393/bcb59453b69e4501b5d4cb6a8184493c

@akankshakumari393
Revert "Revert "Refactor RepositoryServer controller (#2136)" (#2150)"
4ec6907 
This reverts commit 64595c9.
@akankshakumari393
Remove logs from RepositoryServer Controller
dfc83d8 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Remove logs
d9384c9 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Add logs and values that add name and namespace to the logger
6282652 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Run gofmt
6f338d4 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Run gofmt
b46cd28 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Merge branch 'master' into revert-2150-revert-2136-refactor_repositor…
4f412cd 
…yserver
@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 Jul 12, 2023
@akankshakumari393
Handle CR status conditions for Updating Service and pod
df37db7 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Update CR status after updating condition
3d73e75 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
update overall status intead of progress
7b56442 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Handle status condition for All stages in Reconcile function
68982fb 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Run gofmt
71df605 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393 akankshakumari393 changed the title Set CR Condition for Updating Owned Resources Set CR Condition for RepositoryServer Controller Jul 12, 2023
@akankshakumari393 akankshakumari393 changed the title Set CR Condition for RepositoryServer Controller Set RepositoryServerCR Condition Field Jul 12, 2023
@akankshakumari393
handle case for Pod in crashloopbackoff state
da6d0c1 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Remove handling for pod crashloopbackoff status
d4184fd 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Handle failure scenario for starting Repo proxy server
da26965 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Handle failure scenario for server refresh
50bfb6b 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Refactor reconcile function
28cd941 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
typo in using err variable
07dfff6 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Run gofmt
8da5aee 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
Base automatically changed from revert-2150-revert-2136-refactor_repositoryserver to master July 25, 2023 06:28
@akankshakumari393
Merge branch 'master' into handleError_resourcesUpdate
7ec4c2d
@akankshakumari393
Fix lint
f6ba2c3 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Remmove Error handling for RefreshServer and StartRepoProxyServer
006ade2 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Remmove unnecessary log statement
16eed10 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Merge branch 'master' into handleError_resourcesUpdate
231fb88
@akankshakumari393
Set condition to nil on every reconcilition run
2807c17 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Copy link
Contributor Author

akankshakumari393 commented Aug 3, 2023 •
edited

@akankshakumari393 can you check if we can add unit tests for these changes under repositoryserver_controller_test.go?

will create a follow up PR on this.

Kanister automation moved this from In Progress to Reviewer approved Aug 7, 2023
kale-amruta
kale-amruta approved these changes Aug 7, 2023
View reviewed changes
Copy link
Contributor

@kale-amruta kale-amruta left a comment

Choose a reason for hiding this comment

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

lgtm with minor change

viveksinghggits
viveksinghggits reviewed Aug 7, 2023
View reviewed changes
pkg/controllers/repositoryserver/handler.go Outdated Show resolved Hide resolved
pkg/controllers/repositoryserver/handler.go Outdated Show resolved Hide resolved
pkg/controllers/repositoryserver/handler.go
@@ -284,7 +287,7 @@ func (h *RepoServerHandler) preparePodOverride(ctx context.Context) (map[string]
return podOverride, nil
}

func (h *RepoServerHandler) updateServerInfoInCRStatus(ctx context.Context, info crv1alpha1.ServerInfo) error {
func (h *RepoServerHandler) updateServerInfoInCRStatus(ctx context.Context, podName string, serviceName string) error {
h.Logger.Info("Fetch latest version of RepositoryServer to update the ServerInfo in its status")
Copy link
Contributor

Choose a reason for hiding this comment

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

This log is not part of this PR and we don't have to change it now. But it looks like a comment more than a log. And there are other instances of similar logs I see in these files.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should create a task just to correct the logs.

pkg/controllers/repositoryserver/utils.go Outdated Show resolved Hide resolved
pkg/controllers/repositoryserver/handler.go Show resolved Hide resolved
@akankshakumari393
Address review comments
501ae3a 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
@akankshakumari393
Merge branch 'handleError_resourcesUpdate' of github.com:kanisterio/k…
e9ad31f 
…anister into handleError_resourcesUpdate
@akankshakumari393
Add ConsitionStatus struct
d028cfe 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
Kanister automation moved this from Reviewer approved to Review Required Aug 9, 2023
@akankshakumari393
Address review comments Remove ConditionStatus Struct and its usage
c6863a9 
Signed-off-by: Akanksha Kumari <akankshakumari393@gmail.com>
Kanister automation moved this from Review Required to Reviewer approved Aug 9, 2023
@mergify mergify bot merged commit 04fbe86 into master Aug 9, 2023
14 checks passed
Kanister automation moved this from Reviewer approved to Done Aug 9, 2023
@mergify mergify bot deleted the handleError_resourcesUpdate branch August 9, 2023 15:36
This was referenced Aug 10, 2023
Open
Open
@hairyhum hairyhum mentioned this pull request Apr 26, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kale-amruta kale-amruta kale-amruta approved these changes

@viveksinghggits viveksinghggits viveksinghggits approved these changes

@PrasadG193 PrasadG193 Awaiting requested review from PrasadG193

@pavannd1 pavannd1 Awaiting requested review from pavannd1

Assignees

@PrasadG193 PrasadG193

Labels
kueue
Projects
Kanister
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@akankshakumari393 @kale-amruta @viveksinghggits @PrasadG193