Skip to content

Commit

Permalink
fix: Improve code and repair edge case problems with the internal Pod…
Browse files Browse the repository at this point in the history
…Spec updates. (#31)

Significant rewrites for parts of podspec_updates.go to fix broken integration test edge cases

- When deleted, the AuthProxyWorkloads resource hangs around for a few minutes with
   its metadata.DeletionTimestamp set. In some cases, we need to ignore workloads that 
   have already been deleted.
- Correctly determine if a port is in use or not. Fixes an edge case where the ConnectionString on an
  AuthProxyWorkload changes, but the port number remains the same.
- Correctly handle EnvVars for removed AuthProxyWorkloads
- Correctly handle Volumes and VolumeMounts for removed AuthProxyWorkloads

Code cleanup from lint in most of the other files.
- Rename function params in podspec_updates.go for consistency

Add the health check spec to proxy containers.
  • Loading branch information
hessjcg authored Sep 30, 2022
1 parent d182b10 commit fe6ce99
Show file tree
Hide file tree
Showing 5 changed files with 464 additions and 311 deletions.
21 changes: 14 additions & 7 deletions internal/names/names.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func ContainerNameFromNamespacedName(r types.NamespacedName) string {
}

// ContainerName generates a valid name for a corev1.Container object that
// implements this cloudsql instance. Names must be 63 characters or less and
// implements this cloudsql instance. Names must be 63 characters or fewer and
// adhere to the rfc1035/rfc1123 label (DNS_LABEL) format. r.ObjectMeta.Name
// already is required to be 63 characters or less because it is a name. Because
// we are prepending 'csql-' ContainerPrefix as a marker, the generated name with
Expand All @@ -61,29 +61,36 @@ func VolumeName(r *cloudsqlapi.AuthProxyWorkload, inst *cloudsqlapi.InstanceSpec
// to match containers and volumes to configuration. If the names are generated
// differently between one version of the operator and the next, the operator
// will break. Existing workloads will not be correctly updated.
func SafePrefixedName(prefix string, instName string) string {
func SafePrefixedName(prefix, instName string) string {
const maxNameLen = 63 // maximum character limit for a name

containerPrefixLen := len(prefix)

if len(instName)+containerPrefixLen > maxNameLen {
// string shortener that will still produce a name that is still unique
// even though it is truncated.
checksum := hash([]byte(instName))
checksum := mustHash([]byte(instName))
hashSuffix := fmt.Sprintf("-%x", checksum)
hashSuffixLen := len(hashSuffix)
truncateLen := (maxNameLen - hashSuffixLen - containerPrefixLen) / 2
namePrefix := instName[:truncateLen]
nameSuffix := instName[len(instName)-truncateLen:]

return strings.ToLower(strings.Join(
[]string{prefix, namePrefix, nameSuffix, hashSuffix}, ""))
}
return strings.ToLower(prefix + instName)

return strings.ToLower(prefix + instName)
}

// hash simply returns the checksum for a slice of bytes
func hash(bytes []byte) uint32 {
// mustHash simply returns the checksum for a slice of bytes
func mustHash(bytes []byte) uint32 {
h := fnv.New32a()
h.Write(bytes)
i, err := h.Write(bytes)

if err != nil || i != len(bytes) {
panic(fmt.Errorf("unable to calculate mustHash for bytes %v %v", bytes, err))
}

return h.Sum32()
}
10 changes: 3 additions & 7 deletions internal/names/names_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ func TestSafePrefixedName(t *testing.T) {
want: "csql-twas-brillig-and-the-slithy-toves-did-gyre-and-gimble-in-t",
},
{
desc: "truncated difference in middle preserved in hash 1",
desc: "truncated difference in middle preserved in mustHash 1",
name: "twas-brillig-and-the-slithy-toves-1111-did-gyre-and-gimble-in",
want: "csql-twas-brillig-and-the-slit11-did-gyre-and-gimble-in-d0b9860",
},
{
desc: "truncated difference in middle preserved in hash 2",
desc: "truncated difference in middle preserved in mustHash 2",
name: "twas-brillig-and-the-slithy-toves-2222-did-gyre-and-gimble-in",
want: "csql-twas-brillig-and-the-sli2-did-gyre-and-gimble-in-34c209d4",
},
Expand All @@ -70,7 +70,6 @@ func TestSafePrefixedName(t *testing.T) {
}
})
}

}

func TestContainerName(t *testing.T) {
Expand All @@ -80,7 +79,6 @@ func TestContainerName(t *testing.T) {
if want != got {
t.Errorf("got %v, want %v", got, want)
}

}

func TestVolumeName(t *testing.T) {
Expand All @@ -90,10 +88,9 @@ func TestVolumeName(t *testing.T) {
if want != got {
t.Errorf("got %v, want %v", got, want)
}

}

func authProxyWorkload(name string, namespace string) *cloudsqlapi.AuthProxyWorkload {
func authProxyWorkload(name, namespace string) *cloudsqlapi.AuthProxyWorkload {
// Create a CloudSqlInstance that matches the deployment
return &cloudsqlapi.AuthProxyWorkload{
TypeMeta: metav1.TypeMeta{Kind: "AuthProxyWorkload", APIVersion: cloudsqlapi.GroupVersion.String()},
Expand All @@ -109,5 +106,4 @@ func authProxyWorkload(name string, namespace string) *cloudsqlapi.AuthProxyWork
},
Status: cloudsqlapi.AuthProxyWorkloadStatus{},
}

}
Loading

0 comments on commit fe6ce99

Please sign in to comment.