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

Split Sidecar to it's own container #2105

Merged
merged 23 commits into from
May 21, 2024

Conversation

dvaldivia
Copy link
Collaborator

Right now we have a problem where upgrading/downgrading the operator causes all tenants to perform a rolling restart, this is because the sidecar image is the same as the operator.

This PR splits the sidecar into it's own binary, intended to become it's own container image with separate release cycle.

This way, only when there's changes to the sidecar code, we issue a new release and update operator so we can warn users that any given operator release will cause a rolling restart of all tenants.

Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
pkg/resources/statefulsets/minio-sidecar.go Outdated Show resolved Hide resolved
sidecar/Dockerfile Outdated Show resolved Hide resolved
sidecar/cmd/sidecar/app_commands.go Outdated Show resolved Hide resolved
sidecar/cmd/sidecar/main.go Outdated Show resolved Hide resolved
sidecar/cmd/sidecar/main.go Outdated Show resolved Hide resolved
sidecar/pkg/build-constants.go Outdated Show resolved Hide resolved
sidecar/pkg/sidecar/webhook_server.go Outdated Show resolved Hide resolved
sidecar/pkg/validator/validator.go Outdated Show resolved Hide resolved
sidecar/pkg/validator/validator.go Show resolved Hide resolved
testing/common.sh Outdated Show resolved Hide resolved
ramondeklein
ramondeklein previously approved these changes May 8, 2024
Copy link
Contributor

@ramondeklein ramondeklein left a comment

Choose a reason for hiding this comment

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

Do I understand correctly that the sidecar still uses code from the operator (it references github.com/minio/operator in go.mod)? It looks like, although that code is in the same repository, it will use a fixed version that is fetched from GitHub? So, if changes are made to github.com/minio/operator that should be used in the sidecar, then these changes need to be pushed to GitHub and the sidecar dependency should be updated. Am I right?

sidecar/go.mod Outdated Show resolved Hide resolved
sidecar/pkg/validator/validator.go Show resolved Hide resolved
@harshavardhana
Copy link
Member

This way, only when there's changes to the sidecar code, we issue a new release and update operator so we can warn users that any given operator release will cause a rolling restart of all tenants.

Is there no way to avoid this?

@harshavardhana
Copy link
Member

what if we move the sidecar process outside of operator instead move it into a separate project?

@dvaldivia
Copy link
Collaborator Author

@harshavardhana the symbiosis between operator and sidecar is too strong, I anticipate only when we need changes to operator that we will pair them with sidecar changes in that way we can in a single PR perform both

Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
pjuarezd and others added 14 commits May 15, 2024 13:55
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Co-authored-by: Shubhendu <shubhendu.tripathi@gmail.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Add LICENSE and CREDITS to sidecar container
Add goreleaser config for sidecar

Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
pjuarezd
pjuarezd previously approved these changes May 16, 2024
jiuker
jiuker previously approved these changes May 16, 2024
shtripat
shtripat previously approved these changes May 16, 2024
Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

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

LGTM. One open question from @ramondeklein

sidecar/go.mod Outdated Show resolved Hide resolved
@harshavardhana harshavardhana dismissed stale reviews from shtripat, jiuker, and pjuarezd via 2e3ff47 May 16, 2024 07:27
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
```
ERRO Can't read config: can't read viper config: open ./.golangci.yml: no such file or directory
```

Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
```
Vulnerability #1: GO-2024-2824
    Malformed DNS message can cause infinite loop in net
  More info: https://pkg.go.dev/vuln/GO-2024-2824
  Standard library
    Found in: net@go1.22.2
    Fixed in: net@go1.22.3
    Example traces found:
Error:       #1: pkg/sidecar/http_handlers.go:51:60: sidecar.Controller.BucketSrvHandler calls core.services.Delete, which eventually calls net.Dialer.DialContext
Error:       #2: pkg/sidecar/sidecar_utils.go:80:40: sidecar.StartSideCar calls http.Server.ListenAndServe, which calls net.Listen
Error:       #3: cmd/sidecar/main.go:129:31: sidecar.main calls cli.App.Run, which eventually calls net.LookupHost

Your code is affected by 1 vulnerability from the Go standard library.
This scan found no other vulnerabilities in packages you import or modules you
require.
Use '-show verbose' for more details.
```

add binary `minio-operator-sidecar`to gotignore

Signed-off-by: pjuarezd <pjuarezd@users.noreply.github.com>
@harshavardhana
Copy link
Member

BTW lets move CI/CD to go1.22.x // @allanrogerr can you send a fix?

@harshavardhana harshavardhana merged commit 56c1e35 into minio:master May 21, 2024
30 checks passed
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.

None yet

6 participants