Skip to content

Commit

Permalink
fix(discovery): Undo respecting discovery for config poller (#1051)
Browse files Browse the repository at this point in the history
Follow up to #1050
Apparently, being `UP` is predicated on the config poller having successfully run. But now that I made configpoller predicated on being `UP` echo doesn't come up...
There is no harm (just wasted RPS) for disabled instances getting config, so I am undoing this part of the change.

I didn't catch this because I tested with discovery being OFF and then turning it OFF and not the other way around.
  • Loading branch information
marchello2000 committed Nov 2, 2020
1 parent 7042076 commit e614e5c
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.netflix.spinnaker.echo.model.Trigger;
import com.netflix.spinnaker.echo.pipelinetriggers.orca.OrcaService;
import com.netflix.spinnaker.echo.services.Front50Service;
import com.netflix.spinnaker.kork.discovery.DiscoveryStatusListener;
import com.netflix.spinnaker.security.AuthenticatedRequest;
import java.time.Duration;
import java.time.Instant;
Expand Down Expand Up @@ -61,8 +60,6 @@ public class PipelineCache implements MonitoredPoller {
private volatile Boolean running;
private volatile Instant lastPollTimestamp;

private final DiscoveryStatusListener discoveryStatusListener;

@Nullable private volatile List<Pipeline> pipelines;

@Nullable private volatile Map<String, List<Trigger>> triggersByType;
Expand All @@ -72,7 +69,6 @@ public PipelineCache(
@Value("${front50.polling-interval-ms:30000}") int pollingIntervalMs,
@Value("${front50.polling-sleep-ms:100}") int pollingSleepMs,
ObjectMapper objectMapper,
@NonNull DiscoveryStatusListener discoveryStatusListener,
@NonNull Front50Service front50,
@NonNull OrcaService orca,
@NonNull Registry registry) {
Expand All @@ -81,7 +77,6 @@ public PipelineCache(
pollingIntervalMs,
pollingSleepMs,
objectMapper,
discoveryStatusListener,
front50,
orca,
registry);
Expand All @@ -93,15 +88,13 @@ public PipelineCache(
int pollingIntervalMs,
int pollingSleepMs,
ObjectMapper objectMapper,
@NonNull DiscoveryStatusListener discoveryStatusListener,
@NonNull Front50Service front50,
@NonNull OrcaService orca,
@NonNull Registry registry) {
this.objectMapper = objectMapper;
this.executorService = executorService;
this.pollingIntervalMs = pollingIntervalMs;
this.pollingSleepMs = pollingSleepMs;
this.discoveryStatusListener = discoveryStatusListener;
this.front50 = front50;
this.orca = orca;
this.registry = registry;
Expand Down Expand Up @@ -137,14 +130,14 @@ public void run() {
}

private Double getDurationSeconds() {
return (lastPollTimestamp == null || !discoveryStatusListener.isEnabled())
return (lastPollTimestamp == null)
? -1d
: (double) Duration.between(lastPollTimestamp, now()).getSeconds();
}

// VisibleForTesting
void pollPipelineConfigs() {
if (!isRunning() || !discoveryStatusListener.isEnabled()) {
if (!isRunning()) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import com.netflix.spinnaker.echo.model.Trigger
import com.netflix.spinnaker.echo.pipelinetriggers.orca.OrcaService
import com.netflix.spinnaker.echo.services.Front50Service
import com.netflix.spinnaker.echo.test.RetrofitStubs
import com.netflix.spinnaker.kork.discovery.DiscoveryStatusListener
import spock.lang.Shared
import spock.lang.Specification
import spock.lang.Subject
Expand All @@ -38,7 +37,6 @@ class PipelineCacheSpec extends Specification implements RetrofitStubs {
def orca = Mock(OrcaService)
def registry = new NoopRegistry()
def objectMapper = EchoObjectMapper.getInstance()
def activator = Mock(DiscoveryStatusListener)

@Shared
def interval = 30
Expand All @@ -47,7 +45,7 @@ class PipelineCacheSpec extends Specification implements RetrofitStubs {
def sleepMs = 100

@Subject
def pipelineCache = new PipelineCache(Mock(ScheduledExecutorService), interval, sleepMs, objectMapper, activator, front50, orca, registry)
def pipelineCache = new PipelineCache(Mock(ScheduledExecutorService), interval, sleepMs, objectMapper, front50, orca, registry)

def "keeps polling if Front50 returns an error"() {
given:
Expand All @@ -58,8 +56,6 @@ class PipelineCacheSpec extends Specification implements RetrofitStubs {
]
def pipeline = Pipeline.builder().application('application').name('Pipeline').id('P1').build()

activator.isEnabled() >> true

def initialLoad = []
front50.getPipelines() >> initialLoad >> { throw unavailable() } >> [pipelineMap]
pipelineCache.start()
Expand Down

0 comments on commit e614e5c

Please sign in to comment.