Skip to content
Open
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
7 changes: 6 additions & 1 deletion config/configmaps/install-log-regexes-configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,14 @@ data:
installFailingMessage: GCP Precondition Failed
- name: GCPQuotaSSDTotalGBExceeded
searchRegexStrings:
- "Quota \'SSD_TOTAL_GB\' exceeded"
- "Quota 'SSD_TOTAL_GB' exceeded"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure why the test for this succeeds. We are seeing clusters stuck provisioning while the error doesn't get caught - my best bet is on the extra backslashes here, but I can't figure out why the unit tests pass.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's several layers of parsing going on here. I don't have a good explanation, but here's a walkthrough of how I think it should be working:

In the production code path, we read the configmap yaml from bindata, decode it into a runtime object like this before applying it to the target cluster. In the unit test we use the same decoder, but we're reading from the file system rather than from bindata.

In the prod code we're applying the object to the cluster. In the test we're seeding the fake client with it.

We pull the data out of the ConfigMap the same way in both paths (which is kind of the point of the unit test). We should be processing that content as a single monolithic string, wherein the backslashes are (so far) preserved. I think when we do this unmarshal is where they would be interpreted, as the individual regex gets stuffed into an actual element in the []string that is the installLogRegex.SearchRegexStrings array.

But interpreting a backslash preceding a single quote should resolve to just ... a single quote. See here for "proof": notice that the regexes p1 and p2 render the same under %q (which is %s surrounded by double quotes), and thus unsurprisingly yield the same results from Match(), which, again unsurprisingly, matches when the subject string has single quotes, but not when it doesn't, and not when it has explicit backslashes.


We are seeing clusters stuck provisioning while the error doesn't get caught

Can you please clarify? You're saying that you're seeing installation failures with a message that should be getting trapped by this regex, but it's not? It's showing up as a generic failure or something? If you manually change the regex in the configmap, does that fix it? (You may have to scale down hive-operator first so we don't revert your change.)

Copy link
Member Author

@typeid typeid Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please clarify? You're saying that you're seeing installation failures with a message that should be getting trapped by this regex, but it's not?

Right, I was seeing clusters failing with Quota 'SSD_TOTAL_GB' exceeded that didn't get caught/mapped to the error. I'm unable to reproduce this easily though, and I believe just removing the backslashes might work - worst case it doesn't and keeps us in the same state.

Happy to remove this from the PR though if we want to dig further into the issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like we don't know whether removing the backslashes will work... which casts doubt on whether your new thing will work either :)

If you can't easily reproduce the quota problems, I'm not sure how we're going to prove either one.

What we could do (and it's really not a terrible process in general) is add both regexes to your additional-install-log-regexes configmap and try to / wait for repro. (Though I'm guessing that's not really any easier to experiment with than this one as I imagine it's in a git repo somewhere anyway?)

LMK if you're interested in that idea. If not, I don't super have a problem with iterating in this repo. As long as you don't e.g. completely bork the yaml syntax and cause a parsing error, there's really no harm in getting it wrong.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Idly wondering whether the single quotes in the actual message from AWS are "smart" -- something other than ASCII 0x27...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to track down the source of that error message and its printer does come with a salient caveat:

// Format formats the message as a string.
// This method is only intended for human consumption and ignores errors.
// Do not depend on the output being stable. Its output will change across
// different builds of your program, even when using the same version of the
// protobuf module.

installFailingReason: GCPQuotaSSDTotalGBExceeded
installFailingMessage: GCP quota SSD_TOTAL_GB exceeded
- name: GCPQuotaCPUSAllRegionsExceeded
searchRegexStrings:
- "Quota 'CPUS_ALL_REGIONS' exceeded"
installFailingReason: GCPQuotaCPUSAllRegionsExceeded
installFailingMessage: GCP quota CPUS_ALL_REGIONS exceeded
- name: GCPComputeQuota
searchRegexStrings:
- "compute\\.googleapis\\.com/cpus is not available in [a-z0-9-]* because the required number of resources \\([0-9]*\\) is more than"
Expand Down
6 changes: 6 additions & 0 deletions pkg/controller/clusterprovision/installlogmonitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
gcpInvalidProjectIDLog = "blahblah\ntime=\"2020-11-13T16:05:07Z\" level=fatal msg=\"failed to fetch Master Machines: failed to load asset \"Install Config\": platform.gcp.project: Invalid value: \"o-6b20f250\": invalid project ID\nblahblah"
gcpSSDQuotaLog = "blahblah\ntime=\"2021-01-06T03:35:44Z\" level=error msg=\"Error: Error waiting for instance to create: Quota 'SSD_TOTAL_GB' exceeded. Limit: 500.0 in region asia-northeast2.\nblahblah"
gcpCPUQuotaLog = "level=fatal msg=failed to fetch Cluster: failed to fetch dependency of \"Cluster\": failed to generate asset \"Platform Quota Check\": error(MissingQuota): compute.googleapis.com/cpus is not available in us-east1 because the required number of resources (20) is more than remaining quota of 16"
gcpCPUSAllRegionsQuotaLog = "\"Error creating an instance\" err=\"googleapi: Error 403: QUOTA_EXCEEDED - Quota 'CPUS_ALL_REGIONS' exceeded. Limit: 20.0 globally.\""
gcpServiceAccountQuotaLog = "level=fatal msg=failed to fetch Cluster: failed to fetch dependency of \"Cluster\": failed to generate asset \"Platform Quota Check\": error(MissingQuota): iam.googleapis.com/quota/service-account-count is not available in global because the required number of resources (5) is more than remaining quota of 0"
kubeAPIWaitTimeoutLog = "blahblah\ntime=\"2021-01-03T07:04:44Z\" level=fatal msg=\"waiting for Kubernetes API: context deadline exceeded\""
natGatewayLimitExceeded = "blahblah\ntime=\"2021-01-06T03:35:44Z\" level=error msg=\"Error creating NAT Gateway: NatGatewayLimitExceeded: The maximum number of NAT Gateways has been reached.\""
Expand Down Expand Up @@ -372,6 +373,11 @@ func TestParseInstallLog(t *testing.T) {
log: ptr.To(gcpCPUQuotaLog),
expectedReason: "GCPComputeQuotaExceeded",
},
{
name: "GCP CPUS_ALL_REGIONS quota",
log: ptr.To(string(gcpCPUSAllRegionsQuotaLog)),
expectedReason: "GCPQuotaCPUSAllRegionsExceeded",
},
{
name: "GCP service account quota",
log: ptr.To(gcpServiceAccountQuotaLog),
Expand Down
7 changes: 6 additions & 1 deletion pkg/operator/assets/bindata.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.