Skip to content

OLS-1775 : Reconciling app-server and postgres on cert rotation.#1188

Open
sriroopar wants to merge 1 commit intoopenshift:mainfrom
sriroopar:cert-rotation-reconcilation
Open

OLS-1775 : Reconciling app-server and postgres on cert rotation.#1188
sriroopar wants to merge 1 commit intoopenshift:mainfrom
sriroopar:cert-rotation-reconcilation

Conversation

@sriroopar
Copy link
Contributor

@sriroopar sriroopar commented Dec 11, 2025

Description

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up dependent library

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 11, 2025
@openshift-ci
Copy link

openshift-ci bot commented Dec 11, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign bparees for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details 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

@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch 4 times, most recently from f4c5972 to 43b5a83 Compare December 19, 2025 16:13
@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch from 71c1fb9 to 2f2cc67 Compare December 22, 2025 20:58
@sriroopar
Copy link
Contributor Author

sriroopar commented Dec 23, 2025

/retest

2 similar comments
@sriroopar
Copy link
Contributor Author

sriroopar commented Dec 23, 2025

/retest

@sriroopar
Copy link
Contributor Author

sriroopar commented Dec 23, 2025

/retest

@sriroopar sriroopar changed the title [WIP] OLS-1775 : Reconciling app-server and postgres on cert rotation. OLS-1775 : Reconciling app-server and postgres on cert rotation. Dec 23, 2025
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 23, 2025
@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch from 2f2cc67 to b7da0a9 Compare January 3, 2026 04:57
@sriroopar
Copy link
Contributor Author

sriroopar commented Jan 3, 2026

/retest

2 similar comments
@sriroopar
Copy link
Contributor Author

sriroopar commented Jan 3, 2026

/retest

@sriroopar
Copy link
Contributor Author

sriroopar commented Jan 5, 2026

/retest

@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch 3 times, most recently from 83a1c5f to 323302d Compare January 8, 2026 02:37
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2026
@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch from 323302d to e41b3a8 Compare January 8, 2026 03:10
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2026
@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch 3 times, most recently from d72611e to 2757b93 Compare January 14, 2026 13:53
@sriroopar
Copy link
Contributor Author

sriroopar commented Jan 14, 2026

/retest

1 similar comment
@sriroopar
Copy link
Contributor Author

sriroopar commented Jan 15, 2026

/retest

raptorsun
raptorsun reviewed Jan 15, 2026
View reviewed changes
internal/controller/appserver/deployment.go Outdated

// updateOLSDeployment updates the deployment based on CustomResource configuration.
func updateOLSDeployment(r reconciler.Reconciler, ctx context.Context, existingDeployment, desiredDeployment *appsv1.Deployment) error {
func updateOLSDeployment(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig, existingDeployment, desiredDeployment *appsv1.Deployment) error {
Copy link
Contributor

@raptorsun raptorsun Jan 15, 2026

Choose a reason for hiding this comment

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

the argument cr is not used in this function. shall we remove it?

raptorsun
raptorsun reviewed Jan 15, 2026
View reviewed changes
internal/controller/appserver/reconciler.go Outdated
}

err = updateOLSDeployment(r, ctx, existingDeployment, desiredDeployment)
err = updateOLSDeployment(r, ctx, cr, existingDeployment, desiredDeployment)
Copy link
Contributor

@raptorsun raptorsun Jan 15, 2026

Choose a reason for hiding this comment

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

cr is not used in the function updateOLSDeployment

@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch 2 times, most recently from 1889a16 to 5b5e026 Compare January 16, 2026 17:37
@sriroopar
Copy link
Contributor Author

sriroopar commented Jan 16, 2026

/retest

@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch 2 times, most recently from 948b60a to 4e9d2e3 Compare January 23, 2026 14:35
@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch from 4e9d2e3 to b631744 Compare February 2, 2026 13:34
@sriroopar
Copy link
Contributor Author

sriroopar commented Feb 2, 2026

/retest

@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch from b631744 to eb00ff0 Compare February 11, 2026 15:28
@sriroopar
Copy link
Contributor Author

sriroopar commented Feb 11, 2026

/retest

blublinsky
blublinsky reviewed Feb 11, 2026
View reviewed changes
internal/controller/watchers/watchers.go Outdated
return false, nil
})
}

Copy link
Contributor

@blublinsky blublinsky Feb 11, 2026

Choose a reason for hiding this comment

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

This also does not seem correct. I understand that you want to restart things sequentially but there should be a better way of doing this. This seems too much code for wha you want to do

@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch 2 times, most recently from 8029229 to 556cca0 Compare February 16, 2026 22:37
@sriroopar
Copy link
Contributor Author

sriroopar commented Feb 17, 2026

/retest

1 similar comment
@sriroopar
Copy link
Contributor Author

sriroopar commented Feb 17, 2026

/retest

@sriroopar sriroopar force-pushed the cert-rotation-reconcilation branch from 556cca0 to d968fee Compare February 18, 2026 05:04
@openshift-ci
Copy link

openshift-ci bot commented Feb 18, 2026

@sriroopar: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/bundle-e2e-4-20 d968fee link true /test bundle-e2e-4-20

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joshuawilson joshuawilson Awaiting requested review from joshuawilson

2 more reviewers

@raptorsun raptorsun raptorsun left review comments

@blublinsky blublinsky blublinsky left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sriroopar @raptorsun @blublinsky @openshift-merge-robot

Comments