tool(dev): update dev container, makefile, remove object files #609

snguyen64 · 2024-08-15T21:01:21Z

Description

Updates to the dev container to use kind and kind registry for local testing.
Removes generated files including mocks
Update mock generates to be consistent
Update linting to 1.60.3 for workflow and local lint and update deprecated linters.

Related Issue

If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request.

Checklist

I have read the contributing documentation.
I signed and signed-off the commits (git commit -S -s ...). See this documentation on signing commits.
I have correctly attributed the author(s) of the code.
I have tested the changes locally.
I have followed the project's style guidelines.
I have updated the documentation, if necessary.
I have added tests, if applicable.

Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes made.

Additional Notes

Add any additional notes or context about the pull request here.

Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.

nddq · 2024-08-15T21:50:32Z

hmm pretty sure we needed those empty object files, so the linter doesn't complain 🤔

rbtr · 2024-08-16T21:20:05Z

.devcontainer/installMoreTools.sh

 curl -sL https://apt.llvm.org/llvm.sh | sudo bash -s "$LLVM_VERSION"
+echo 'export PATH=$PATH:/usr/lib/llvm-14/bin' >> ~/.profile


this differs from how we install llvm in the bulid containers, should we make them consistent?

should the devcontainer be the same base mariner image as the build container?

hm im actually not too familiar with the pros and cons, i was thinking jammy would be more known and easy to develop on, but i dont have much experience with mariner. I can update this if you think it is better, then we can be more consistent

I think that the closer the dev / prod impedence match is, the more likely you are to catch compatibility issues. I'd say go with mariner if it's usable in dev.

I did mess around for a little bit with this image, seems that some devcontainer features (docker-in-docker, kind, kubernetes-cli, and github-cli) don't work by default so we would have to set that up
Im not sure if it's worth it to figure out the configuration for the above missing features
If you guys think it's worth it then i can play around with it a bit more and figure something out

In this current setup, there is docker-in-docker and kind which works -> if we were to use mariner im not sure if this would work - im thinking it would need to mount to a socket?

I was just envisioning it as installing dependencies, running out make commands that generate retina images, and deploying to kind cluster, so i don't see as much importance in matching this container image with the retina container image

rbtr · 2024-08-16T21:21:09Z

hack/tools/tools.go

@@ -13,7 +13,6 @@ $ go install ...
 package tools

 import (
-	_ "github.com/golangci/golangci-lint/cmd/golangci-lint"


oh i see the makefile changes now. not convinced that that's better, though

reverted and updated to match golangci-lint versions and updated workflow/config - this seemed to solve the issue

rbtr · 2024-08-16T21:22:14Z

Makefile

@@ -524,6 +524,24 @@ quick-build:
 	$(MAKE) retina-image PLATFORM=linux/amd64 BUILDX_ACTION=--push
 	$(MAKE) retina-operator-image PLATFORM=linux/amd64 BUILDX_ACTION=--push

+.PHONY: quick-build-devcontainer
+quick-build-devcontainer: # buildx push doesnt work for some reason in kind


instead of *-devcontainer targets, maybe we could have a REGISTRY=local flag or something like that?

i had these separate to not affect existing commands, and some of the steps here are pretty much tied to devcontainer (like the registry:port)
I can update these though if you think it is better

i prefer some kind of flag/option rather than duplicate targets if everything else is identical, but i guess that can be less discoverable also. no strong opinion, your call

My preference is for dev-related things to have a fast alias and prod-build things to have more flags and knobs. Yes, there's always shell aliases, but it is nice when the standard dev experience doesn't require creating aliases to be productive. It's easy to put whatever flags / tuning are needed in CI, so I'm not as bothered by that (and +1 to the discoverability problem for devex).

Makefile

.github/workflows/golangci-lint.yaml

nddq · 2024-09-04T21:02:06Z

@snguyen64 i just remember one thing, we might need those. o files for vendoring as well since they are embedded in the generated ebpf wrapper files. go mod vendor will not success IIRC, but you can verify it

github-actions · 2024-10-05T00:21:29Z

This PR will be closed in 7 days due to inactivity.

github-actions · 2024-10-13T00:23:50Z

Pull request closed due to inactivity.

snguyen64 force-pushed the feat/dev-update branch 2 times, most recently from 90bf985 to 041eebb Compare August 15, 2024 21:12

snguyen64 changed the title ~~tools(dev): update dev container, makefile, remove object files~~ tool(dev): update dev container, makefile, remove object files Aug 15, 2024

snguyen64 marked this pull request as ready for review August 15, 2024 21:15

snguyen64 requested a review from a team as a code owner August 15, 2024 21:15

snguyen64 requested review from kamilprz and ritwikranjan August 15, 2024 21:15

snguyen64 force-pushed the feat/dev-update branch 12 times, most recently from 0f5a249 to 1418ed7 Compare August 16, 2024 20:56

rbtr reviewed Aug 16, 2024

View reviewed changes

snguyen64 force-pushed the feat/dev-update branch 9 times, most recently from 3df6854 to 41a9d4c Compare August 20, 2024 22:49

snguyen64 force-pushed the feat/dev-update branch from 220fb4b to 7e7cbd2 Compare August 21, 2024 19:33

snguyen64 marked this pull request as draft August 27, 2024 23:18

snguyen64 force-pushed the feat/dev-update branch 6 times, most recently from d22a563 to 50148ec Compare August 28, 2024 17:55

snguyen64 added 8 commits August 28, 2024 18:01

tool(dev): update dev container, makefile, remove object files

c97f57d

add workflow step before linting to generate object files

36ed368

match golangci versions and add gitignore for mock generated

da779a6

fix workflow

e6450ce

update for windows

6ef5317

test windows update

0e4c7c0

update to build and generate

4238cec

replace mockgen pull with go.mod

741a99e

snguyen64 force-pushed the feat/dev-update branch from 50148ec to 3b9fea9 Compare August 28, 2024 18:03

setup tools

9e7e1f4

snguyen64 force-pushed the feat/dev-update branch 4 times, most recently from 56564ee to 59bd248 Compare August 28, 2024 20:32

update mock generation to be consistent

965863b

snguyen64 force-pushed the feat/dev-update branch from 59bd248 to 965863b Compare August 28, 2024 21:01

snguyen64 marked this pull request as ready for review August 28, 2024 21:43

github-actions bot added the meta/waiting-for-author Blocked and waiting on the author label Oct 5, 2024

github-actions bot closed this Oct 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tool(dev): update dev container, makefile, remove object files #609

tool(dev): update dev container, makefile, remove object files #609

snguyen64 commented Aug 15, 2024 •

edited

Loading

nddq commented Aug 15, 2024

rbtr Aug 16, 2024

rbtr Aug 16, 2024

snguyen64 Aug 16, 2024

timraymond Aug 19, 2024

snguyen64 Aug 20, 2024

rbtr Aug 16, 2024

rbtr Aug 16, 2024

snguyen64 Aug 20, 2024

rbtr Aug 16, 2024

snguyen64 Aug 16, 2024

rbtr Aug 20, 2024

timraymond Aug 20, 2024

nddq commented Sep 4, 2024

github-actions bot commented Oct 5, 2024

github-actions bot commented Oct 13, 2024

		curl -sL https://apt.llvm.org/llvm.sh \| sudo bash -s "$LLVM_VERSION"
		echo 'export PATH=$PATH:/usr/lib/llvm-14/bin' >> ~/.profile

tool(dev): update dev container, makefile, remove object files #609

tool(dev): update dev container, makefile, remove object files #609

Conversation

snguyen64 commented Aug 15, 2024 • edited Loading

Description

Related Issue

Checklist

Screenshots (if applicable) or Testing Completed

Additional Notes

nddq commented Aug 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nddq commented Sep 4, 2024

github-actions bot commented Oct 5, 2024

github-actions bot commented Oct 13, 2024

snguyen64 commented Aug 15, 2024 •

edited

Loading