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

fix: middleware message handling #28

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

mindhells
Copy link
Member

@mindhells mindhells commented Jan 29, 2025

This PR aims to fix some issues related to middleware message handling:

  • error tolerance on message handling from homing pigeon to the 1st middleware of the chain
  • add a retry policy to handle external middleware service discontinuity or wait IPC process initialisation
  • wait for service to be ready in case of punctual unavailability
  • middlewares implemented using unimplemented interface
  • fix: pass the next middleware response up through the chain
  • apply same retry and waitForReady policies

Additionally:

  • default image user and group will be set to 1000
  • multi-arch image build (arm and amd)

@mindhells mindhells requested review from sviridoff and bvis January 29, 2025 11:52
_, err = (*b.client).Handle(context.Background(), req)
return nil, err
klog.V(0).Info("processing next middleware")
ctxTimeout, cancelTimeout := context.WithTimeout(context.Background(), 12*time.Second)

Choose a reason for hiding this comment

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

Shouldn't we add the retry config here too?

Copy link
Member Author

Choose a reason for hiding this comment

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

default retry policy is added when creating the client for the next middleware in this file, you mean that?

Choose a reason for hiding this comment

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

Now I've seen the defaultRetryPolicy variable use, thanks :-)

klog.Errorf("Failed to remove socket: %v", err)
}

err = os.MkdirAll(filepath.Dir(socket), 0775)

Choose a reason for hiding this comment

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

Where is the socket folder created now?

Copy link
Member Author

Choose a reason for hiding this comment

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

we don't need to, do we?

Choose a reason for hiding this comment

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

I think so, you don't know the socket path you are receiving, so, How do you know it is created?

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.

3 participants