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

breakout envoyinit #10499

Merged
merged 5 commits into from
Jan 24, 2025
Merged

breakout envoyinit #10499

merged 5 commits into from
Jan 24, 2025

Conversation

lgadban
Copy link
Contributor

@lgadban lgadban commented Jan 23, 2025

Prepares the envoyinit component for removal of projects/gloo

Still needs work, notably: #10491

@lgadban lgadban force-pushed the law/breakout-envoyinit branch from 30b0439 to 4f37ebb Compare January 23, 2025 18:55
Comment on lines 3 to 4
const GlooSystem = "gloo-system"
const GlooFed = "gloo-system"
Copy link
Contributor

Choose a reason for hiding this comment

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

where are these used? remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leftover cleanup, fixed

//go:generate sh -c "echo '\n\t// gloo filter types' >> filter_types.gen.go"
//go:generate sh -c "go list github.com/solo-io/gloo/projects/gloo/pkg/api/external/envoy/... | xargs -n1 -I{} echo '\t_ \"{}\"' >> filter_types.gen.go"
//go:generate sh -c "echo ')' >> filter_types.gen.go"
// TODO: commenting out go generate for now, it can't exist until we figure out how to get the types from within
Copy link
Contributor

Choose a reason for hiding this comment

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

i tried doing this locally and it seemed to work.. what are you seeing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yuval-k and I spent some time on this and was seeing the same behavior.

I think we'll need to move forward with this hack until we properly fix envoyinit

@timflannagan
Copy link
Member

@lgadban Do we still need to pin the gloo envoy image in the deployment chart with these changes, or is this a precursor to removing gloo and eventually removing that hack that deploys a hardcoded gloo envoy image?

@lgadban
Copy link
Contributor Author

lgadban commented Jan 23, 2025

or is this a precursor to removing gloo and eventually removing that hack that deploys a hardcoded gloo envoy image?

@timflannagan This is correct, it's the precursor to removing gloo; for now we'll keep the envoy image hardcoded
Technically, since envoyinit is buildable though at this point, we might be able to re-add the envoyinit build and tag back into the Makefile.
Is this preferable to be done before your helm chart work goes in?

@@ -32,6 +31,8 @@ const (
ServerInfoPath = "server_info"
)

var EnvoyAdminPort uint32 = 19000
Copy link
Member

Choose a reason for hiding this comment

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

Does this still need to be exported? I don't see any other references in the diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

leftover from copy/paste; fixed

@lgadban lgadban added this pull request to the merge queue Jan 24, 2025
Merged via the queue into main with commit 04e883c Jan 24, 2025
6 checks passed
@lgadban lgadban deleted the law/breakout-envoyinit branch January 24, 2025 15:44
timflannagan added a commit to timflannagan/kgateway that referenced this pull request Jan 24, 2025
This was a temporary hack needed until kgateway-dev#10499 landed in main.

Signed-off-by: timflannagan <timflannagan@gmail.com>
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.

3 participants