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

Issue 657: Providing support to update controller service annotations #658

Merged
merged 3 commits into from
Mar 7, 2023

Conversation

anishakj
Copy link
Contributor

@anishakj anishakj commented Mar 6, 2023

Change log description

Added support to update the annotations and labels at run time for controller service

Purpose of the change

Fixes #657

What the code does

In controller reconcile service, if there is change in service like annotations, labels it will be updated

How to verify it

Verified by adding new annotations at run time

@anishakj anishakj requested a review from jkhalack March 6, 2023 12:05
@anishakj anishakj marked this pull request as ready for review March 6, 2023 12:05
@anishakj anishakj requested a review from gaddamas March 6, 2023 12:48
} else {
return err
}

Copy link

Choose a reason for hiding this comment

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

(nitpicking) Could you remove the extra empty lines from the new code to conform to the file formatting? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

err = r.Client.Create(context.TODO(), service)
if err != nil && !errors.IsAlreadyExists(err) {
return err
}

Copy link

Choose a reason for hiding this comment

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

This line still looks strange

Copy link
Contributor Author

Choose a reason for hiding this comment

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

here are you suggesting to remove the IsAlreadyExists check?

Copy link

Choose a reason for hiding this comment

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

I mean the empty line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@anishakj anishakj merged commit a1dcfec into master Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Operator is not updating the controller service if new annotations are added
3 participants