Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/clusterdefaults/clusterdefaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func (ct ClusterType) String() string {
}
}

// TODO(#91): Maybe I'm missing something, but this manager/detector/applicator abstraction
// TODO(ROX-34499): Maybe I'm missing something, but this manager/detector/applicator abstraction
// seems massively over-engineered since the is only one concrete implementation. AFAICT this
// could all be just a single function that the deployer calls with log, kubeconfig, resources,
// exposure and portForward.
Expand Down
6 changes: 3 additions & 3 deletions internal/deployer/deploy_via_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ func (d *Deployer) waitForCentralReady(ctx context.Context, timeout time.Duratio
}
}

// TODO(#91): using `kubectl wait` (which in turn - I hope - uses a watch) instead of
// TODO(ROX-34499): using `kubectl wait` (which in turn - I hope - uses a watch) instead of
// polling would allow us to not waste time here
time.Sleep(checkInterval)
}
Expand Down Expand Up @@ -614,7 +614,7 @@ func (d *Deployer) configureCentralEndpoint(ctx context.Context, exposure string
return fmt.Errorf("failed to get LoadBalancer endpoint: %w", err)
}
// Remove https:// prefix if present (waitForLoadBalancer returns https://ip:443)
// TODO(#91): This is silly, why add these affixes there, and then strip here?!
// TODO(ROX-34499): This is silly, why add these affixes there, and then strip here?!
d.centralEndpoint = strings.TrimPrefix(endpoint, "https://")
d.centralEndpoint = strings.TrimSuffix(d.centralEndpoint, ":443")
d.centralEndpoint = d.centralEndpoint + ":443"
Expand Down Expand Up @@ -841,7 +841,7 @@ func (d *Deployer) waitForSecuredClusterReady(ctx context.Context, timeout time.
}

// collector seems to be crashing on some local cluster types/versions.
// TODO(#91): skip the check only on local clusters, then?
// TODO(ROX-34499): skip the check only on local clusters, then?
}

if allReady {
Expand Down
2 changes: 1 addition & 1 deletion internal/deployer/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (

// runKubectl is a thin wrapper around k8s.RunKubectl that injects the deployer's logger.
//
// TODO(#91): perhaps this should not return a pointer so that the lack of nil checks elsewhere
// TODO(ROX-34499): perhaps this should not return a pointer so that the lack of nil checks elsewhere
// is robust?
func (d *Deployer) runKubectl(ctx context.Context, opts k8s.KubectlOptions) (*k8s.KubectlResult, error) {
return k8s.RunKubectl(ctx, d.logger, opts)
Expand Down
2 changes: 1 addition & 1 deletion internal/deployer/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func (d *Deployer) identifyCRDFileNames(bundleDir string) ([]string, error) {
return nil
}

// TODO(#91): The following detection logic does not seem particularly robust. We should
// TODO(ROX-34499): The following detection logic does not seem particularly robust. We should
// probably parse the YAML and check api group and kind fields.
name := strings.ToLower(info.Name())
if strings.Contains(name, "customresourcedefinition") ||
Expand Down
12 changes: 6 additions & 6 deletions internal/deployer/operator_olm.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (d *Deployer) checkOLMInstalled(ctx context.Context) error {
}

for _, crd := range requiredCRDs {
// TODO(#91): actually this is not the right way to check whether it's safe to create a resource of a given kind.
// TODO(ROX-34499): actually this is not the right way to check whether it's safe to create a resource of a given kind.
// A CRD can be present, but still being loaded or end up not accepted by the API server.
// Instead we should use the `kubectl api-resources` subcommand which exposes the status we're looking for.
_, err := d.runKubectl(ctx, k8s.KubectlOptions{
Expand Down Expand Up @@ -128,7 +128,7 @@ func (d *Deployer) createCatalogSource(ctx context.Context, indexImage string) e
},
}

// TODO(#91): Add security context config if supported.
// TODO(ROX-34499): Add security context config if supported.
if hasSecurityContextConfig {
spec := catalogSource["spec"].(map[string]interface{})
spec["grpcPodConfig"] = map[string]interface{}{
Expand Down Expand Up @@ -162,7 +162,7 @@ func (d *Deployer) catalogSourceSupportsSecurityContextConfig(ctx context.Contex
return false, err
}

// TODO(#91): this is overly optimistic and would incorrectly succeed if an api version
// TODO(ROX-34499): this is overly optimistic and would incorrectly succeed if an api version
// that contains this had "serving: false"
return strings.Contains(result.Stdout, "securityContextConfig"), nil
}
Expand Down Expand Up @@ -257,7 +257,7 @@ func (d *Deployer) waitForAndApproveInstallPlan(ctx context.Context) error {
}

if time.Since(start) >= timeout {
// TODO(#91): some more info on what was wrong would be useful: a dump of the
// TODO(ROX-34499): some more info on what was wrong would be useful: a dump of the
// subscription or at least its name so that the user can investigate
return errors.New("timeout waiting for InstallPlan to be created")
}
Expand Down Expand Up @@ -329,7 +329,7 @@ func (d *Deployer) waitForCSVSuccess(ctx context.Context) error {
time.Sleep(5 * time.Second)
}

// TODO(#91): same as above
// TODO(ROX-34499): same as above
return fmt.Errorf("timeout waiting for CSV to succeed")
}

Expand All @@ -353,7 +353,7 @@ func (d *Deployer) detectOperatorDeploymentMode(ctx context.Context) (bool, Oper
result, err := d.runKubectl(ctx, k8s.KubectlOptions{
Args: []string{"get", "deployment", operatorDeploymentName, "-n", operatorNamespace, "-o", "jsonpath={.metadata.labels}"},
})
// TODO(#91): This is not very robust. Better retrieve a specific label in the `get`
// TODO(ROX-34499): This is not very robust. Better retrieve a specific label in the `get`
// command?
if err == nil && strings.Contains(result.Stdout, "olm.owner") {
return true, OperatorModeOLM
Expand Down
2 changes: 1 addition & 1 deletion internal/deployer/override.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func setNestedValue(m map[string]interface{}, path string, value interface{}) er
if existingMap, isMap := existing.(map[string]interface{}); isMap {
current = existingMap
} else {
// TODO(#91): shouldn't this be an error instead? I think it's more likely someone
// TODO(ROX-34499): shouldn't this be an error instead? I think it's more likely someone
// made a typo...
// Existing value is not a map, we need to replace it
newMap := make(map[string]interface{})
Expand Down
2 changes: 1 addition & 1 deletion internal/portforward/portforward.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func (m *Manager) Start(namespace, serviceName string, remotePort, preferredLoca
if err == nil {
syscall.Kill(-pgid, syscall.SIGTERM)
}
// TODO(#91): AFAICT this can get stuck forever if the process blocks SIGTERM...
// TODO(ROX-34499): AFAICT this can get stuck forever if the process blocks SIGTERM...
cmd.Wait()
}
return "", fmt.Errorf("port-forward did not become ready")
Expand Down
Loading