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.

Sign up for GitHub

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

Jump to bottom

refactor: server side apply over create / update #3075

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open

refactor: server side apply over create / update #3075

Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
466e27a
changing create/updates to use ssa
AustinAbro321 Oct 7, 2024
3e2fd88
WIP, more to do but tests passing
AustinAbro321 Oct 9, 2024
b978890
merge
AustinAbro321 Oct 17, 2024
bdb756d
tests passing
AustinAbro321 Oct 17, 2024
9d1a35d
patch-> apply
AustinAbro321 Oct 17, 2024
9f757e9
remove patches
AustinAbro321 Oct 17, 2024
0219569
tests passing
AustinAbro321 Oct 17, 2024
0b17664
merge
AustinAbro321 Oct 17, 2024
21d8c04
fix debug
AustinAbro321 Oct 17, 2024
4516b5f
health checks
AustinAbro321 Oct 17, 2024
905711d
pointer
AustinAbro321 Oct 18, 2024
71eb153
using correct svc again
AustinAbro321 Oct 18, 2024
7c38c13
ssa in zarf
AustinAbro321 Oct 18, 2024
00868f4
add field manager
AustinAbro321 Oct 18, 2024
b121ded
adopt existing resources
AustinAbro321 Oct 18, 2024
e83f6e6
comment
AustinAbro321 Oct 18, 2024
570b244
delete comment
AustinAbro321 Oct 18, 2024
5736f56
add comment
AustinAbro321 Oct 18, 2024
ca48559
make injection pod and service easier to read
AustinAbro321 Oct 18, 2024
52643f7
injector
AustinAbro321 Oct 18, 2024
72f07c9
remove redundant test
AustinAbro321 Oct 18, 2024
9a898dc
Merge branch 'main' into try-out-ssa
AustinAbro321 Oct 18, 2024
5cae3e5
fix post render logic
AustinAbro321 Oct 21, 2024
16ed156
fix post render logic
AustinAbro321 Oct 21, 2024
f1d1886
better error
AustinAbro321 Oct 23, 2024
9f1d13d
add namespace and api version
AustinAbro321 Oct 23, 2024
593c317
move namespaces back to create / update
AustinAbro321 Oct 24, 2024
7543833
create->apply
AustinAbro321 Oct 24, 2024
e825432
newline
AustinAbro321 Oct 24, 2024
aa8c22b
consistent creation
AustinAbro321 Oct 24, 2024
dc8bc71
verbiage
AustinAbro321 Oct 24, 2024
dd5f752
Merge branch 'main' into try-out-ssa
AustinAbro321 Oct 24, 2024
f5e30a8
Merge branch 'try-out-ssa' of github.com:zarf-dev/zarf into try-out-ssa
AustinAbro321 Oct 24, 2024
94376a5
Merge branch 'main' into try-out-ssa
AustinAbro321 Oct 29, 2024
ad93bcb
merge
AustinAbro321 Oct 31, 2024
a00784d
field manager name
AustinAbro321 Nov 1, 2024
c15887e
no nil needed
AustinAbro321 Nov 1, 2024
e1ff23f
remove maps equal logic
AustinAbro321 Nov 1, 2024
ac86e82
newline
AustinAbro321 Nov 1, 2024
e6d25d3
Merge branch 'main' into try-out-ssa
AustinAbro321 Nov 1, 2024
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
2 changes: 1 addition & 1 deletion src/internal/agent/hooks/utils_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type admissionTest struct {

func createTestClientWithZarfState(ctx context.Context, t *testing.T, state *types.ZarfState) *cluster.Cluster {
t.Helper()
c := &cluster.Cluster{Clientset: fake.NewSimpleClientset()}
c := &cluster.Cluster{Clientset: fake.NewClientset()}
stateData, err := json.Marshal(state)
require.NoError(t, err)

Expand Down
55 changes: 11 additions & 44 deletions src/internal/packager/helm/post-render.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"bytes"
"context"
"fmt"
"maps"
"os"
"path/filepath"
"slices"
Expand All @@ -21,11 +20,11 @@ import (
"github.com/zarf-dev/zarf/src/pkg/utils"
"github.com/zarf-dev/zarf/src/types"
"helm.sh/helm/v3/pkg/releaseutil"
corev1 "k8s.io/api/core/v1"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/restmapper"
"sigs.k8s.io/yaml"

corev1 "k8s.io/api/core/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -164,49 +163,17 @@ func (r *renderer) adoptAndUpdateNamespaces(ctx context.Context) error {
if err != nil {
return err
}
// TODO: Refactor as error is not checked instead of checking for not found error.
currentRegistrySecret, _ := c.Clientset.CoreV1().Secrets(name).Get(ctx, config.ZarfImagePullSecretName, metav1.GetOptions{})
sameSecretData := maps.EqualFunc(currentRegistrySecret.Data, validRegistrySecret.Data, func(v1, v2 []byte) bool { return bytes.Equal(v1, v2) })
if currentRegistrySecret.Name != config.ZarfImagePullSecretName || !sameSecretData {
Copy link
Member

Choose a reason for hiding this comment

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

What is the effect of removing this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current code only updates the secret if the name or the data is different. Maybe it was there to preserve labels and annotations? I'm not sure, but that's no longer a worry with apply at least.

Additionally, the current code updates the git server secret if the registry secret has a different name or different data, this seems like it was a bug with indentation, but I'm not 100% sure. I don't see any reason for that logic.

Still, I should have made a unit test to verify the behavior, I will do that now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I am realizing the existing logic is flawed. I don't think there's much value in testing it. I could separate the new logic and write a test for it, but since it's just two apply calls, I'd pretty much just be testing that the fake client works.

Additionally, after evaluating further, there is no reason to dog sled errors here. We always want both of these secrets created

err := func() error {
_, err := c.Clientset.CoreV1().Secrets(validRegistrySecret.Namespace).Create(ctx, validRegistrySecret, metav1.CreateOptions{})
if err != nil && !kerrors.IsAlreadyExists(err) {
return err
}
if err == nil {
return nil
}
_, err = c.Clientset.CoreV1().Secrets(validRegistrySecret.Namespace).Update(ctx, validRegistrySecret, metav1.UpdateOptions{})
if err != nil {
return err
}
return nil
}()
if err != nil {
message.WarnErrf(err, "Problem creating registry secret for the %s namespace", name)
l.Warn("problem creating registry secret", "namespace", name, "error", err.Error())
}
_, err = c.Clientset.CoreV1().Secrets(*validRegistrySecret.Namespace).Apply(ctx, validRegistrySecret, metav1.ApplyOptions{Force: true, FieldManager: "zarf"})
if err != nil {
message.WarnErrf(err, "Problem creating registry secret for the %s namespace", name)
l.Warn("problem creating registry secret", "namespace", name, "error", err.Error())
}

// Create or update the zarf git server secret
gitServerSecret := c.GenerateGitPullCreds(name, config.ZarfGitServerSecretName, r.state.GitServer)
err = func() error {
_, err := c.Clientset.CoreV1().Secrets(gitServerSecret.Namespace).Create(ctx, gitServerSecret, metav1.CreateOptions{})
if err != nil && !kerrors.IsAlreadyExists(err) {
return err
}
if err == nil {
return nil
}
_, err = c.Clientset.CoreV1().Secrets(gitServerSecret.Namespace).Update(ctx, gitServerSecret, metav1.UpdateOptions{})
if err != nil {
return err
}
return nil
}()
if err != nil {
message.WarnErrf(err, "Problem creating git server secret for the %s namespace", name)
l.Warn("problem creating git server secret", "namespace", name, "error", err.Error())
}
gitServerSecret := c.GenerateGitPullCreds(name, config.ZarfGitServerSecretName, r.state.GitServer)
_, err = c.Clientset.CoreV1().Secrets(*gitServerSecret.Namespace).Apply(ctx, gitServerSecret, metav1.ApplyOptions{Force: true, FieldManager: "zarf"})
if err != nil {
message.WarnErrf(err, "Problem creating git server secret for the %s namespace", name)
l.Warn("problem creating git server secret", "namespace", name, "error", err.Error())
}
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion src/internal/packager2/load_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestPackageFromSourceOrCluster(t *testing.T) {
require.Equal(t, "test", pkg.Metadata.Name)

c := &cluster.Cluster{
Clientset: fake.NewSimpleClientset(),
Clientset: fake.NewClientset(),
}
_, err = c.RecordPackageDeployment(ctx, pkg, nil)
require.NoError(t, err)
Expand Down
Loading