Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(build): fix debug dockerfile and debug skaffold makefile targets #71

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

pmalek
Copy link
Member

@pmalek pmalek commented Mar 28, 2024

What this PR does / why we need it:

Fixes debug Dockerfile after changes in https://github.com/Kong/gateway-operator-archive/issues/1554

Instead of relying on third_party/ dir it hardcodes the dlv version used in the dockerfile.

We could potentially add a renovate comment to the line in the dockerfile so that the version is bumped automatically as soon as renovate is enabled on this repo.

@pmalek pmalek self-assigned this Mar 28, 2024
@pmalek pmalek requested a review from a team as a code owner March 28, 2024 16:44
@pmalek pmalek enabled auto-merge (squash) March 28, 2024 16:44
@pmalek pmalek changed the title fix(build): fix debug dockerfile fix(build): fix debug dockerfile and debug skaffold makefile targets Mar 28, 2024
Copy link
Member

@programmer04 programmer04 left a comment

Choose a reason for hiding this comment

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

On the clean cluster and repo is it expected to run

make skaffold

before running

make debug.skaffold

for the first time? Otherwise, it doesn't work.

Furthemore, it hangs (log from gateway-operator-controller-manager-f6c5fd8d9-xrx9x)

2024-03-28T17:13:09Z debug layer=debugger continuing
2024-03-28T17:13:09Z debug layer=debugger ContinueOnce
2024-03-28T17:13:10.033Z - INFO - SETUP - starting controller manager - {"release": "v1.2.1-16-g366fe8c-debug", "repo": "[email protected]:Kong/gateway-operator.git", "commit": "366fe8c"}
2024-03-28T17:13:10.033Z - INFO - SETUP - development mode enabled
2024-03-28T17:13:10.033Z - INFO - SETUP - leader election disabled
2024-03-28T17:13:10.041Z - ERROR - SETUP - failed setting up controllers - {"error": "missing a required CRD: gateway.networking.k8s.io/v1, Resource=gatewayclasses"}

@pmalek
Copy link
Member Author

pmalek commented Mar 29, 2024

@programmer04 This stems from the fact that we require Gateway API CRDs when they respective controllers are enabled.

// These checks prevent controller-runtime spamming in logs about failing
// to get informer from cache.
// This way we only ever check the CRD once and issue clear log entry about
// particular CRD missing.
// Also this makes it possible to specify more complex boolean conditions
// whether to check for particular CRD or not, and also makes it possible to
// specify several CRDs to be checked for existence, which are required
// for specific controller to work.
crdChecks := []struct {
Condition bool
GVRs []schema.GroupVersionResource
}{
{
Condition: c.GatewayControllerEnabled || c.DataPlaneBlueGreenControllerEnabled || c.DataPlaneControllerEnabled,
GVRs: []schema.GroupVersionResource{
operatorv1beta1.DataPlaneGVR(),
},
},
{
Condition: c.GatewayControllerEnabled || c.ControlPlaneControllerEnabled,
GVRs: []schema.GroupVersionResource{
operatorv1beta1.ControlPlaneGVR(),
},
},
{
Condition: c.GatewayControllerEnabled,
GVRs: []schema.GroupVersionResource{
{
Group: gatewayv1.SchemeGroupVersion.Group,
Version: gatewayv1.SchemeGroupVersion.Version,
Resource: "gatewayclasses",
},
{
Group: gatewayv1.SchemeGroupVersion.Group,
Version: gatewayv1.SchemeGroupVersion.Version,
Resource: "gateways",
},
},
},
}
checker := crdChecker{client: mgr.GetClient()}
for _, check := range crdChecks {
if !check.Condition {
continue
}
for _, gvr := range check.GVRs {
if ok, err := checker.CRDExists(gvr); err != nil {
return nil, err
} else if !ok {
return nil, fmt.Errorf("missing a required CRD: %v", gvr)
}
}
}

I'd prefer not to bundle these in the debug kustomization as that will become a thing that we need to maintain and secondly this would overwrite what users have in their clusters already installed. WDYT?

@programmer04
Copy link
Member

Ok, seems resonable

@pmalek pmalek merged commit 4e516db into main Mar 29, 2024
13 checks passed
@pmalek pmalek deleted the fix-debug-dockerfile branch March 29, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants