Skip to content

Commit

Permalink
🐛 Manager leader election: Don't reset restcfg UserAgent
Browse files Browse the repository at this point in the history
In pkg.LeaderElection.NewResourceLock we call rest.AddUserAgent
which resets the restcfgs useragent and sets it to the default one plus
a suffix. This resets whatever was originally set as UserAgent and since
we do not copy our restcfg before passing it in there, this leads to the
UserAgent being set to the leader-election one globally.
  • Loading branch information
alvaroaleman committed Feb 27, 2021
1 parent e388e1e commit 51f0fde
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 3 deletions.
6 changes: 3 additions & 3 deletions pkg/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,9 +308,9 @@ func New(config *rest.Config, options Options) (Manager, error) {
}

// Create the resource lock to enable leader election)
leaderConfig := config
if options.LeaderElectionConfig != nil {
leaderConfig = options.LeaderElectionConfig
leaderConfig := options.LeaderElectionConfig
if leaderConfig == nil {
leaderConfig = rest.CopyConfig(config)
}
resourceLock, err := options.newResourceLock(leaderConfig, recorderProvider, leaderelection.Options{
LeaderElection: options.LeaderElection,
Expand Down
14 changes: 14 additions & 0 deletions pkg/manager/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,20 @@ var _ = Describe("manger.Manager", func() {
close(done)
})

It("should not manipulate the provided config", func() {
originalCfg := rest.CopyConfig(cfg)
// The options object is shared by multiple tests, copy it
// into our scope so we manipulate it for this testcase only
options := options
options.newResourceLock = nil
m, err := New(cfg, options)
Expect(err).NotTo(HaveOccurred())
for _, cb := range callbacks {
cb(m)
}
Expect(m.GetConfig()).To(Equal(originalCfg))
})

It("should stop when context is cancelled", func(done Done) {
m, err := New(cfg, options)
Expect(err).NotTo(HaveOccurred())
Expand Down

0 comments on commit 51f0fde

Please sign in to comment.