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

Revert workaround for mdsd bug #2471

Merged
merged 1 commit into from
Oct 17, 2023
Merged

Revert workaround for mdsd bug #2471

merged 1 commit into from
Oct 17, 2023

Conversation

s-fairchild
Copy link
Collaborator

@s-fairchild s-fairchild commented Oct 18, 2022 •
edited
Loading

Which issue this PR addresses:

https://issues.redhat.com/browse/ARO-1535

What this PR does / why we need it:

Remove workaround that reset log permissions to syslog. Once Microsoft fixes race condition in this card this PR will be ready to merge.

This PR reverts a workaround (resetting log permissions to syslog) for a MDSD race condition.
When this problem occurs, odd and hard to track down logging behavior can occur. In this case of this ICM log messages were sent to dgrep, but not kusto. Alerts were received that AzSecPack was not running, but dgrep showed proof it was running.

On the RP VMSS an MDSD race event was occurring. Threads were using different uids to create files/directories, resulting in MDSD losing permission to those directories. See ICM comments for detailed error message examples.

The version of MDSD affected was not found in notes or attached log mesages.

Test plan for issue:

Tested by deploying pull request to INT. Deployment and E2E passed

Is there any documentation that needs to be updated for this PR?

No

@s-fairchild
Copy link
Collaborator Author

@microsoft-github-policy-service agree company="Red Hat"

s-amann
s-amann previously approved these changes Oct 18, 2022
View reviewed changes
Copy link
Contributor

@s-amann s-amann left a comment

Choose a reason for hiding this comment

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

lgtm, nice work! Let's continue to hold merging this until MSFT has their fix in place.

ross-bryan
ross-bryan previously approved these changes Oct 18, 2022
View reviewed changes
@github-actions github-actions bot added the needs-rebase branch needs a rebase label Oct 19, 2022
@github-actions
Copy link

Please rebase pull request.

@s-fairchild s-fairchild dismissed stale reviews from ross-bryan and s-amann via b5088a4 October 19, 2022 17:26
@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Oct 19, 2022
ross-bryan
ross-bryan previously approved these changes Oct 20, 2022
View reviewed changes
@github-actions github-actions bot added the needs-rebase branch needs a rebase label Oct 24, 2022
@github-actions
Copy link

Please rebase pull request.

@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Oct 26, 2022
@cadenmarchese cadenmarchese added the chainsaw Pull requests or issues owned by Team Chainsaw label Jul 17, 2023
@github-actions github-actions bot added needs-rebase branch needs a rebase and removed needs-rebase branch needs a rebase labels Jul 17, 2023
@github-actions
Copy link

Please rebase pull request.

@s-fairchild
Copy link
Collaborator Author

This pull request was merged fixing the MDSD bug.

I'm trying to determine which image version will contain this fix. Once we have confirmed the MDSD image will be updated to include the fix we can merge/rollout this change.

@s-fairchild
Copy link
Collaborator Author

Fix has been merged and can be expected in MDSD rollout version 1.28. This PR should be merged after we've updated to the 1.28 (or later) image.

@bennerv
Copy link
Collaborator

bennerv commented Aug 31, 2023

@s-fairchild is this ready to go with our most recent MDSD bump? #3106

@s-fairchild
Copy link
Collaborator Author

s-fairchild commented Aug 31, 2023 •
edited
Loading

@bennerv Yes, it's ready now that MDSD has been updated.

@s-fairchild
Revert workaround for mdsd bug
bf35b71 
Remove workaround that reset log permissions to syslog.
Once Microsoft fixes race condition in https://dev.azure.com/msazure/One/_workitems/edit/12512148 this PR will be ready to merge.
@github-actions github-actions bot removed the needs-rebase branch needs a rebase label Sep 19, 2023
@s-fairchild
Copy link
Collaborator Author

Deployment and E2E passed in INT

@s-amann s-amann added next-release To be included in the next RP release rollout and removed hold Hold labels Sep 21, 2023
@hlipsig
Copy link
Contributor

hlipsig commented Oct 4, 2023

Dependency for this card is completed, but card is over a year old. Closing and @s-fairchild if this is needed please re-open.

@hlipsig hlipsig closed this Oct 4, 2023
@s-fairchild s-fairchild reopened this Oct 12, 2023
@s-fairchild
Copy link
Collaborator Author

@hlipsig Confirming that the dependency card had been completed took some time. After this I needed to test in INT. This PR is ready to merge for the next release.

@cadenmarchese cadenmarchese dismissed SudoBrendan’s stale review October 17, 2023 14:02

Comments resolved, and CI is passing

@cadenmarchese cadenmarchese merged commit 1844f2d into Azure:master Oct 17, 2023
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cadenmarchese cadenmarchese cadenmarchese approved these changes

@s-amann s-amann s-amann approved these changes

@ross-bryan ross-bryan ross-bryan left review comments

@SudoBrendan SudoBrendan SudoBrendan left review comments

@jewzaam jewzaam Awaiting requested review from jewzaam jewzaam is a code owner

@m1kola m1kola Awaiting requested review from m1kola

@bennerv bennerv Awaiting requested review from bennerv bennerv is a code owner

@hawkowl hawkowl Awaiting requested review from hawkowl hawkowl is a code owner

@rogbas rogbas Awaiting requested review from rogbas rogbas is a code owner

@petrkotas petrkotas Awaiting requested review from petrkotas petrkotas is a code owner

@darthhexx darthhexx Awaiting requested review from darthhexx

@jharrington22 jharrington22 Awaiting requested review from jharrington22 jharrington22 is a code owner

@cblecker cblecker Awaiting requested review from cblecker cblecker is a code owner

@facchettos facchettos Awaiting requested review from facchettos

@UlrichSchlueter UlrichSchlueter Awaiting requested review from UlrichSchlueter UlrichSchlueter is a code owner

@ellis-johnson ellis-johnson Awaiting requested review from ellis-johnson

@yjst2012 yjst2012 Awaiting requested review from yjst2012 yjst2012 is a code owner

@anshulvermapatel anshulvermapatel Awaiting requested review from anshulvermapatel anshulvermapatel is a code owner

@Shivkumar13 Shivkumar13 Awaiting requested review from Shivkumar13

@hlipsig hlipsig Awaiting requested review from hlipsig hlipsig is a code owner

Assignees
No one assigned
Labels
chainsaw Pull requests or issues owned by Team Chainsaw next-release To be included in the next RP release rollout
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@s-fairchild @SudoBrendan @bennerv @hlipsig @ross-bryan @cadenmarchese @s-amann