Skip to content

Conversation

anandsyncs
Copy link
Contributor

@anandsyncs anandsyncs commented Aug 20, 2025

Summary

Version Handling Improvements:

  • Refactored ValidateSearchImageVersion to accept a version string instead of extracting it internally, and renamed the internal method to getMongotVersion for clarity. The helper now sets mdbSearch.Status.Version to the determined version.
  • Updated the logic for extracting the image tag from a container image to use a new extractImageTag helper function, ensuring the version is parsed correctly from the image string.

Testing Updates:

  • Modified the TestMongoDBSearchReconcile_Success test to use the new operator config and to assert that Status.Version is set to the expected value after reconciliation.

Proof of Work

Tests Pass

Checklist

  • Have you linked a jira ticket and/or is the ticket in the title?
  • Have you checked whether your jira ticket required DOCSP changes?
  • Have you added changelog file?

@anandsyncs anandsyncs added the skip-changelog Use this label in Pull Request to not require new changelog entry file label Aug 20, 2025
@anandsyncs anandsyncs marked this pull request as ready for review August 20, 2025 11:55
@anandsyncs anandsyncs requested a review from a team as a code owner August 20, 2025 11:55
@anandsyncs anandsyncs requested review from fealebenpae, MaciejKaras and lsierant and removed request for a team and MaciejKaras August 20, 2025 11:55
Copy link
Contributor

@fealebenpae fealebenpae left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Normally I would have expected us to modify the MongoDBSearch.UpdateStatus() method in mongodbsearch_types.go to set the version field on the status, but I recognize it would be very tricky to actually do that because we don't have the information to compute the implicit version there. I suggest just adding a comment in MongoDBSearchReconcileHelper.reconcile() where setting the version in the status subresource to explain we do it there because we need access to the OperatorSearchConfig struct to compute the version, and we can't access it from MongoDBSearch.UpdateStatus().

@anandsyncs
Copy link
Contributor Author

Looks good to me!

Normally I would have expected us to modify the MongoDBSearch.UpdateStatus() method in mongodbsearch_types.go to set the version field on the status, but I recognize it would be very tricky to actually do that because we don't have the information to compute the implicit version there. I suggest just adding a comment in MongoDBSearchReconcileHelper.reconcile() where setting the version in the status subresource to explain we do it there because we need access to the OperatorSearchConfig struct to compute the version, and we can't access it from MongoDBSearch.UpdateStatus().

Thanks for the review, I tried to do it via MongoDBSearch.UpdateStatus() but it was "tricky" even with some refactoring.

@lsierant lsierant force-pushed the search/public-preview branch from d8e903f to 446c8b3 Compare August 26, 2025 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Use this label in Pull Request to not require new changelog entry file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants