net/redisclient: fix nil pointer dereference

[AlexanderYastrebov]

Fix use of logger before initialization.

Followup on #2261

[AlexanderYastrebov]

I think we should remove this probe from the client - it simply delays creation of the client and client will recover after redis is up. Also client provides RingAvailable method that could be used to check if ring is up.

We also check AddrUpdater once at startup

skipper/skipper.go

Lines 1717 to 1730 in 37c474a

	_, err = redisOptions.AddrUpdater()
	if err != nil {
	log.Errorf("Failed to update redis addresses from kubernetes: %v", err)
	return err
	}
	} else if redisOptions != nil && o.SwarmRedisEndpointsRemoteURL != "" {
	log.Infof("Use remote address %s to fetch updates redis shards", o.SwarmRedisEndpointsRemoteURL)
	redisOptions.AddrUpdater = getRemoteURLRedisAddrUpdater(o.SwarmRedisEndpointsRemoteURL)
	_, err = redisOptions.AddrUpdater()
	if err != nil {
	log.Errorf("Failed to update redis addresses from URL: %v", err)
	return err
	}

which might be a bigger problem - imagine permanent redis failure prevents new pods startup leading to cascading failure. I think its better to fail open/close with ratelimits then fail startup when redis is not avaialble.

[MustafaSaber]

I get your point but 10 sec extra startup time is not that much and it will take longer with container restart. Maybe we don't need to fail startup but it's still good to have the retries, wdyt?

[szuecs]

which might be a bigger problem - imagine permanent redis failure prevents new pods startup leading to cascading failure. I think its better to fail open/close with ratelimits then fail startup when redis is not avaialble.

I disagree here and you should page if this is a permanent issue. A new bad deployment will block and you get a page.

[AlexanderYastrebov]

I get your point but 10 sec extra startup time is not that much and it will take longer with container restart.

Check the code, its just a delay, it only logs an error.

[szuecs]

👍

[AlexanderYastrebov]

👍