crl-updater: duplicate shard 0 as shard NumShards (#7008)
When crl-updater produces a CRL with shard index 0, have it also produce an identical CRL with shard index NumShards (e.g. 128). This is the first step towards having it only produce shards numbered 1 through NumShards, i.e. transition from using 0-indexing to 1-indexing. We want to do this because various aspects of Golang and gRPC cannot tell the difference between "this struct/message has no shard index set" and "this struct/message has shard index 0 set". Part of https://github.com/letsencrypt/boulder/issues/7007
This commit is contained in:
		
							parent
							
								
									75acd40df1
								
							
						
					
					
						commit
						63319d8cd0
					
				| 
						 | 
					@ -264,6 +264,18 @@ func (cu *crlUpdater) tickIssuer(ctx context.Context, atTime time.Time, issuerNa
 | 
				
			||||||
					shardIdx: idx,
 | 
										shardIdx: idx,
 | 
				
			||||||
					err:      cu.tickShardWithRetry(ctx, atTime, issuerNameID, idx, shardMap[idx]),
 | 
										err:      cu.tickShardWithRetry(ctx, atTime, issuerNameID, idx, shardMap[idx]),
 | 
				
			||||||
				}
 | 
									}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
									// We want to renumber shard 0 to also be shard numShards (e.g. 128).
 | 
				
			||||||
 | 
									// To facilitate that transition, produce the same CRL with both shard
 | 
				
			||||||
 | 
									// indices.
 | 
				
			||||||
 | 
									// TODO(#7007): Collapse this when we don't need to produce both anymore.
 | 
				
			||||||
 | 
									if idx == 0 {
 | 
				
			||||||
 | 
										out <- shardResult{
 | 
				
			||||||
 | 
											shardIdx: cu.numShards,
 | 
				
			||||||
 | 
											err:      cu.tickShardWithRetry(ctx, atTime, issuerNameID, cu.numShards, shardMap[idx]),
 | 
				
			||||||
 | 
										}
 | 
				
			||||||
 | 
									}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
				cancel()
 | 
									cancel()
 | 
				
			||||||
			}
 | 
								}
 | 
				
			||||||
		}
 | 
							}
 | 
				
			||||||
| 
						 | 
					@ -275,13 +287,15 @@ func (cu *crlUpdater) tickIssuer(ctx context.Context, atTime time.Time, issuerNa
 | 
				
			||||||
		go shardWorker(shardIdxs, shardResults)
 | 
							go shardWorker(shardIdxs, shardResults)
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
						// TODO(#7007): Iterate from 1 to numShards instead of 0 to numShards-1.
 | 
				
			||||||
	for shardIdx := 0; shardIdx < cu.numShards; shardIdx++ {
 | 
						for shardIdx := 0; shardIdx < cu.numShards; shardIdx++ {
 | 
				
			||||||
		shardIdxs <- shardIdx
 | 
							shardIdxs <- shardIdx
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
	close(shardIdxs)
 | 
						close(shardIdxs)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	var errShards []int
 | 
						var errShards []int
 | 
				
			||||||
	for i := 0; i < cu.numShards; i++ {
 | 
						// TODO(#7007): Reduce this to cu.numShards when we stop producing shard 0.
 | 
				
			||||||
 | 
						for i := 0; i < cu.numShards+1; i++ {
 | 
				
			||||||
		res := <-shardResults
 | 
							res := <-shardResults
 | 
				
			||||||
		if res.err != nil {
 | 
							if res.err != nil {
 | 
				
			||||||
			cu.log.AuditErrf(
 | 
								cu.log.AuditErrf(
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -316,15 +316,15 @@ func TestTickIssuer(t *testing.T) {
 | 
				
			||||||
	test.AssertNotError(t, err, "building test crlUpdater")
 | 
						test.AssertNotError(t, err, "building test crlUpdater")
 | 
				
			||||||
 | 
					
 | 
				
			||||||
	// An error that affects all shards should have every shard reflected in the
 | 
						// An error that affects all shards should have every shard reflected in the
 | 
				
			||||||
	// combined error message.
 | 
						// combined error message, including the bonus shard with index=numShards.
 | 
				
			||||||
	err = cu.tickIssuer(context.Background(), cu.clk.Now(), e1.NameID())
 | 
						err = cu.tickIssuer(context.Background(), cu.clk.Now(), e1.NameID())
 | 
				
			||||||
	test.AssertError(t, err, "database error")
 | 
						test.AssertError(t, err, "database error")
 | 
				
			||||||
	test.AssertContains(t, err.Error(), "2 shards failed")
 | 
						test.AssertContains(t, err.Error(), "3 shards failed")
 | 
				
			||||||
	test.AssertContains(t, err.Error(), "[0 1]")
 | 
						test.AssertContains(t, err.Error(), "[0 1 2]")
 | 
				
			||||||
	test.AssertEquals(t, len(mockLog.GetAllMatching("Generating CRL failed:")), 2)
 | 
						test.AssertEquals(t, len(mockLog.GetAllMatching("Generating CRL failed:")), 3)
 | 
				
			||||||
	test.AssertMetricWithLabelsEquals(t, cu.tickHistogram, prometheus.Labels{
 | 
						test.AssertMetricWithLabelsEquals(t, cu.tickHistogram, prometheus.Labels{
 | 
				
			||||||
		"issuer": "(TEST) Elegant Elephant E1", "result": "failed",
 | 
							"issuer": "(TEST) Elegant Elephant E1", "result": "failed",
 | 
				
			||||||
	}, 2)
 | 
						}, 3)
 | 
				
			||||||
	test.AssertMetricWithLabelsEquals(t, cu.tickHistogram, prometheus.Labels{
 | 
						test.AssertMetricWithLabelsEquals(t, cu.tickHistogram, prometheus.Labels{
 | 
				
			||||||
		"issuer": "(TEST) Elegant Elephant E1 (Overall)", "result": "failed",
 | 
							"issuer": "(TEST) Elegant Elephant E1 (Overall)", "result": "failed",
 | 
				
			||||||
	}, 1)
 | 
						}, 1)
 | 
				
			||||||
| 
						 | 
					@ -359,7 +359,7 @@ func TestTick(t *testing.T) {
 | 
				
			||||||
	test.AssertContains(t, err.Error(), "2 issuers failed")
 | 
						test.AssertContains(t, err.Error(), "2 issuers failed")
 | 
				
			||||||
	test.AssertContains(t, err.Error(), "(TEST) Elegant Elephant E1")
 | 
						test.AssertContains(t, err.Error(), "(TEST) Elegant Elephant E1")
 | 
				
			||||||
	test.AssertContains(t, err.Error(), "(TEST) Radical Rhino R3")
 | 
						test.AssertContains(t, err.Error(), "(TEST) Radical Rhino R3")
 | 
				
			||||||
	test.AssertEquals(t, len(mockLog.GetAllMatching("Generating CRL failed:")), 4)
 | 
						test.AssertEquals(t, len(mockLog.GetAllMatching("Generating CRL failed:")), 6)
 | 
				
			||||||
	test.AssertEquals(t, len(mockLog.GetAllMatching("Generating CRLs for issuer failed:")), 2)
 | 
						test.AssertEquals(t, len(mockLog.GetAllMatching("Generating CRLs for issuer failed:")), 2)
 | 
				
			||||||
	test.AssertMetricWithLabelsEquals(t, cu.tickHistogram, prometheus.Labels{
 | 
						test.AssertMetricWithLabelsEquals(t, cu.tickHistogram, prometheus.Labels{
 | 
				
			||||||
		"issuer": "(TEST) Elegant Elephant E1 (Overall)", "result": "failed",
 | 
							"issuer": "(TEST) Elegant Elephant E1 (Overall)", "result": "failed",
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in New Issue