Skip to content

Commit 6262b93

Browse files
Switching default to Docker Hub and Minor code refactoring (#930)
* Minor refactoring Signed-off-by: Pravin Pushkar <[email protected]> * Swithcing default to dockerhub and adding some unit tests Signed-off-by: Pravin Pushkar <[email protected]> * changing redis to rejson Signed-off-by: Pravin Pushkar <[email protected]> * Revert "changing redis to rejson" This reverts commit aed3cf7. Signed-off-by: Pravin Pushkar <[email protected]> * nit fixes Signed-off-by: Pravin Pushkar <[email protected]> * modifying unit test to accomodate rejson to redis Signed-off-by: Pravin Pushkar <[email protected]>
1 parent 63edec5 commit 6262b93

File tree

4 files changed

+233
-39
lines changed

4 files changed

+233
-39
lines changed

.github/workflows/self_hosted_e2e.yaml

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,18 +77,18 @@ jobs:
7777
echo "Found $DASHBOARD_VERSION"
7878
shell: bash
7979
- name: Run E2E tests with GHCR
80-
if: github.event.schedule != '0 0 * * *'
80+
# only runs once in a day
81+
if: github.event.schedule == '0 0 * * *'
82+
env:
83+
DAPR_DEFAULT_IMAGE_REGISTRY: ghcr
8184
run: |
8285
export TEST_OUTPUT_FILE=$GITHUB_WORKSPACE/test-e2e-standalone.json
8386
echo "TEST_OUTPUT_FILE=$TEST_OUTPUT_FILE" >> $GITHUB_ENV
8487
export GITHUB_TOKEN=${{ secrets.GITHUB_TOKEN }}
8588
make e2e-build-run-sh
8689
shell: bash
8790
- name: Run E2E tests with Docker hub
88-
# only runs once in a day
89-
if: github.event.schedule == '0 0 * * *'
90-
env:
91-
DAPR_DEFAULT_IMAGE_REGISTRY: dockerhub
91+
if: github.event.schedule != '0 0 * * *'
9292
run: |
9393
export TEST_OUTPUT_FILE=$GITHUB_WORKSPACE/test-e2e-standalone.json
9494
echo "TEST_OUTPUT_FILE=$TEST_OUTPUT_FILE" >> $GITHUB_ENV

pkg/standalone/standalone.go

Lines changed: 51 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@ type initInfo struct {
119119
imageRegistryURL string
120120
}
121121

122+
type daprImageInfo struct {
123+
ghcrImageName string
124+
dockerHubImageName string
125+
imageRegistryURL string
126+
imageRegistryName string
127+
}
128+
122129
// Check if the previous version is already installed.
123130
func isBinaryInstallationRequired(binaryFilePrefix, installDir string) (bool, error) {
124131
binaryPath := binaryFilePath(installDir, binaryFilePrefix)
@@ -289,14 +296,13 @@ func runZipkin(wg *sync.WaitGroup, errorChan chan<- error, info initInfo) {
289296
// do not create container again if it exists.
290297
args = append(args, "start", zipkinContainerName)
291298
} else {
292-
if info.imageRegistryURL != "" && info.imageRegistryURL != ghcrURI && info.imageRegistryURL != "docker.io" {
293-
imageName = fmt.Sprintf(privateRegTemplateString, info.imageRegistryURL, zipkinGhcrImageName)
294-
} else if defaultImageRegistryName == githubContainerRegistryName && info.imageRegistryURL == "" {
295-
imageName = fmt.Sprintf("%s/%s", ghcrURI, zipkinGhcrImageName)
296-
} else if defaultImageRegistryName == dockerContainerRegistryName && info.imageRegistryURL == "" {
297-
imageName = zipkinDockerImageName
298-
} else {
299-
err = fmt.Errorf("either %s or Env variable %s not set properly", "--image-registry", "DAPR_DEFAULT_IMAGE_REGISTRY")
299+
imageName, err = resolveImageURI(daprImageInfo{
300+
ghcrImageName: zipkinGhcrImageName,
301+
dockerHubImageName: zipkinDockerImageName,
302+
imageRegistryURL: info.imageRegistryURL,
303+
imageRegistryName: defaultImageRegistryName,
304+
})
305+
if err != nil {
300306
errorChan <- err
301307
return
302308
}
@@ -356,14 +362,13 @@ func runRedis(wg *sync.WaitGroup, errorChan chan<- error, info initInfo) {
356362
// do not create container again if it exists.
357363
args = append(args, "start", redisContainerName)
358364
} else {
359-
if info.imageRegistryURL != "" && info.imageRegistryURL != ghcrURI && info.imageRegistryURL != "docker.io" {
360-
imageName = fmt.Sprintf(privateRegTemplateString, info.imageRegistryURL, redisGhcrImageName)
361-
} else if defaultImageRegistryName == githubContainerRegistryName && info.imageRegistryURL == "" {
362-
imageName = fmt.Sprintf("%s/%s", ghcrURI, redisGhcrImageName)
363-
} else if defaultImageRegistryName == dockerContainerRegistryName && info.imageRegistryURL == "" {
364-
imageName = redisDockerImageName
365-
} else {
366-
err = fmt.Errorf("either %s or Env variable %s not set properly", "--image-registry", "DAPR_DEFAULT_IMAGE_REGISTRY")
365+
imageName, err = resolveImageURI(daprImageInfo{
366+
ghcrImageName: redisGhcrImageName,
367+
dockerHubImageName: redisDockerImageName,
368+
imageRegistryURL: info.imageRegistryURL,
369+
imageRegistryName: defaultImageRegistryName,
370+
})
371+
if err != nil {
367372
errorChan <- err
368373
return
369374
}
@@ -410,24 +415,23 @@ func runPlacementService(wg *sync.WaitGroup, errorChan chan<- error, info initIn
410415

411416
placementContainerName := utils.CreateContainerName(DaprPlacementContainerName, info.dockerNetwork)
412417

413-
var image string
418+
image, err := resolveImageURI(daprImageInfo{
419+
ghcrImageName: daprGhcrImageName,
420+
dockerHubImageName: daprDockerImageName,
421+
imageRegistryURL: info.imageRegistryURL,
422+
imageRegistryName: defaultImageRegistryName,
423+
})
424+
if err != nil {
425+
errorChan <- err
426+
return
427+
}
428+
image = getPlacementImageWithTag(image, info.runtimeVersion)
414429

415-
if info.imageRegistryURL != "" && info.imageRegistryURL != ghcrURI && info.imageRegistryURL != "docker.io" {
416-
image = getPlacementImageWithTag(daprGhcrImageName, info.runtimeVersion)
417-
image = fmt.Sprintf(privateRegTemplateString, info.imageRegistryURL, image)
418-
} else if defaultImageRegistryName == githubContainerRegistryName && info.imageRegistryURL == "" {
419-
image = getPlacementImageWithTag(daprGhcrImageName, info.runtimeVersion)
420-
image = fmt.Sprintf("%s/%s", ghcrURI, image)
430+
if defaultImageRegistryName == githubContainerRegistryName {
421431
if !TryPullImage(image) {
422432
print.InfoStatusEvent(os.Stdout, "Placement image not found in Github container registry, pulling it from Docker Hub")
423433
image = getPlacementImageWithTag(daprDockerImageName, info.runtimeVersion)
424434
}
425-
} else if defaultImageRegistryName == dockerContainerRegistryName && info.imageRegistryURL == "" {
426-
image = getPlacementImageWithTag(daprDockerImageName, info.runtimeVersion)
427-
} else {
428-
err := fmt.Errorf("either %s or Env variable %s not set properly", "--image-registry", "DAPR_DEFAULT_IMAGE_REGISTRY")
429-
errorChan <- err
430-
return
431435
}
432436

433437
exists, err := confirmContainerIsRunningOrExists(placementContainerName, false)
@@ -1062,3 +1066,21 @@ func getPlacementImageWithTag(name, version string) string {
10621066
}
10631067
return fmt.Sprintf("%s:%s", name, version)
10641068
}
1069+
1070+
func resolveImageURI(imageInfo daprImageInfo) (string, error) {
1071+
if imageInfo.imageRegistryURL != "" {
1072+
if imageInfo.imageRegistryURL == ghcrURI || imageInfo.imageRegistryURL == "docker.io" {
1073+
err := fmt.Errorf("flag %s not set correctly", "--image-registry")
1074+
return "", err
1075+
}
1076+
return fmt.Sprintf(privateRegTemplateString, imageInfo.imageRegistryURL, imageInfo.ghcrImageName), nil
1077+
}
1078+
switch imageInfo.imageRegistryName {
1079+
case dockerContainerRegistryName:
1080+
return imageInfo.dockerHubImageName, nil
1081+
case githubContainerRegistryName:
1082+
return fmt.Sprintf("%s/%s", ghcrURI, imageInfo.ghcrImageName), nil
1083+
default:
1084+
return "", errors.New("imageRegistryName not set correctly")
1085+
}
1086+
}

pkg/standalone/standalone_test.go

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,175 @@ spec: {}
6060

6161
os.Remove(testFile)
6262
}
63+
64+
func TestResolveImageWithGHCR(t *testing.T) {
65+
expectedRedisImageName := "ghcr.io/dapr/3rdparty/redis"
66+
expectedZipkinImageName := "ghcr.io/dapr/3rdparty/zipkin"
67+
expectedPlacementImageName := "ghcr.io/dapr/dapr"
68+
69+
redisImageInfo := daprImageInfo{
70+
ghcrImageName: redisGhcrImageName,
71+
dockerHubImageName: redisDockerImageName,
72+
imageRegistryURL: "",
73+
imageRegistryName: "ghcr",
74+
}
75+
zipkinImageInfo := daprImageInfo{
76+
ghcrImageName: zipkinGhcrImageName,
77+
dockerHubImageName: zipkinDockerImageName,
78+
imageRegistryURL: "",
79+
imageRegistryName: "ghcr",
80+
}
81+
placementImageInfo := daprImageInfo{
82+
ghcrImageName: daprGhcrImageName,
83+
dockerHubImageName: daprDockerImageName,
84+
imageRegistryURL: "",
85+
imageRegistryName: "ghcr",
86+
}
87+
88+
tests := []struct {
89+
name string
90+
args daprImageInfo
91+
expect string
92+
expectErr bool
93+
}{
94+
{"Test Redis image name", redisImageInfo, expectedRedisImageName, false},
95+
{"Test Zipkin image name", zipkinImageInfo, expectedZipkinImageName, false},
96+
{"Test Dapr image name", placementImageInfo, expectedPlacementImageName, false},
97+
}
98+
99+
for _, test := range tests {
100+
t.Run(test.name, func(t *testing.T) {
101+
got, err := resolveImageURI(test.args)
102+
assert.Equal(t, test.expectErr, err != nil)
103+
assert.Equal(t, test.expect, got)
104+
})
105+
}
106+
}
107+
108+
func TestResolveImageWithDockerHub(t *testing.T) {
109+
expectedRedisImageName := "redis"
110+
expectedZipkinImageName := "openzipkin/zipkin"
111+
expectedPlacementImageName := "daprio/dapr"
112+
113+
redisImageInfo := daprImageInfo{
114+
ghcrImageName: redisGhcrImageName,
115+
dockerHubImageName: redisDockerImageName,
116+
imageRegistryURL: "",
117+
imageRegistryName: "dockerhub",
118+
}
119+
zipkinImageInfo := daprImageInfo{
120+
ghcrImageName: zipkinGhcrImageName,
121+
dockerHubImageName: zipkinDockerImageName,
122+
imageRegistryURL: "",
123+
imageRegistryName: "dockerhub",
124+
}
125+
placementImageInfo := daprImageInfo{
126+
ghcrImageName: daprGhcrImageName,
127+
dockerHubImageName: daprDockerImageName,
128+
imageRegistryURL: "",
129+
imageRegistryName: "dockerhub",
130+
}
131+
132+
tests := []struct {
133+
name string
134+
args daprImageInfo
135+
expect string
136+
expectErr bool
137+
}{
138+
{"Test Redis image name", redisImageInfo, expectedRedisImageName, false},
139+
{"Test Zipkin image name", zipkinImageInfo, expectedZipkinImageName, false},
140+
{"Test Dapr image name", placementImageInfo, expectedPlacementImageName, false},
141+
}
142+
143+
for _, test := range tests {
144+
t.Run(test.name, func(t *testing.T) {
145+
got, err := resolveImageURI(test.args)
146+
assert.Equal(t, test.expectErr, err != nil)
147+
assert.Equal(t, test.expect, got)
148+
})
149+
}
150+
}
151+
152+
func TestResolveImageWithPrivateRegistry(t *testing.T) {
153+
expectedRedisImageName := "docker.io/username/dapr/3rdparty/redis"
154+
expectedZipkinImageName := "docker.io/username/dapr/3rdparty/zipkin"
155+
expectedPlacementImageName := "docker.io/username/dapr/dapr"
156+
157+
redisImageInfo := daprImageInfo{
158+
ghcrImageName: redisGhcrImageName,
159+
dockerHubImageName: redisDockerImageName,
160+
imageRegistryURL: "docker.io/username",
161+
imageRegistryName: "",
162+
}
163+
zipkinImageInfo := daprImageInfo{
164+
ghcrImageName: zipkinGhcrImageName,
165+
dockerHubImageName: zipkinDockerImageName,
166+
imageRegistryURL: "docker.io/username",
167+
imageRegistryName: "",
168+
}
169+
placementImageInfo := daprImageInfo{
170+
ghcrImageName: daprGhcrImageName,
171+
dockerHubImageName: daprDockerImageName,
172+
imageRegistryURL: "docker.io/username",
173+
imageRegistryName: "",
174+
}
175+
176+
tests := []struct {
177+
name string
178+
args daprImageInfo
179+
expect string
180+
expectErr bool
181+
}{
182+
{"Test Redis image name", redisImageInfo, expectedRedisImageName, false},
183+
{"Test Zipkin image name", zipkinImageInfo, expectedZipkinImageName, false},
184+
{"Test Dapr image name", placementImageInfo, expectedPlacementImageName, false},
185+
}
186+
187+
for _, test := range tests {
188+
t.Run(test.name, func(t *testing.T) {
189+
got, err := resolveImageURI(test.args)
190+
assert.Equal(t, test.expectErr, err != nil)
191+
assert.Equal(t, test.expect, got)
192+
})
193+
}
194+
}
195+
196+
func TestResolveImageErr(t *testing.T) {
197+
redisImageInfo := daprImageInfo{
198+
ghcrImageName: redisGhcrImageName,
199+
dockerHubImageName: redisDockerImageName,
200+
imageRegistryURL: "docker.io",
201+
imageRegistryName: "",
202+
}
203+
zipkinImageInfo := daprImageInfo{
204+
ghcrImageName: zipkinGhcrImageName,
205+
dockerHubImageName: zipkinDockerImageName,
206+
imageRegistryURL: ghcrURI,
207+
imageRegistryName: "",
208+
}
209+
placementImageInfo := daprImageInfo{
210+
ghcrImageName: daprGhcrImageName,
211+
dockerHubImageName: daprDockerImageName,
212+
imageRegistryURL: "",
213+
imageRegistryName: "value_other_than_dockerhub_or_ghcr",
214+
}
215+
216+
tests := []struct {
217+
name string
218+
args daprImageInfo
219+
expect string
220+
expectErr bool
221+
}{
222+
{"Test Redis image name", redisImageInfo, "", true},
223+
{"Test Zipkin image name", zipkinImageInfo, "", true},
224+
{"Test Dapr image name", placementImageInfo, "", true},
225+
}
226+
227+
for _, test := range tests {
228+
t.Run(test.name, func(t *testing.T) {
229+
got, err := resolveImageURI(test.args)
230+
assert.Equal(t, test.expectErr, err != nil)
231+
assert.Equal(t, test.expect, got)
232+
})
233+
}
234+
}

utils/utils.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,12 +236,12 @@ func GetDefaultRegistry(githubContainerRegistryName, dockerContainerRegistryName
236236
val := strings.ToLower(os.Getenv("DAPR_DEFAULT_IMAGE_REGISTRY"))
237237
switch val {
238238
case "":
239-
print.InfoStatusEvent(os.Stdout, "Container images will be pulled from GitHub container registry")
240-
return githubContainerRegistryName, nil
241-
case dockerContainerRegistryName:
242-
print.InfoStatusEvent(os.Stdout, "Container images will be pulled from DockerHub")
239+
print.InfoStatusEvent(os.Stdout, "Container images will be pulled from Docker Hub")
243240
return dockerContainerRegistryName, nil
241+
case githubContainerRegistryName:
242+
print.InfoStatusEvent(os.Stdout, "Container images will be pulled from Dapr GitHub container registry")
243+
return githubContainerRegistryName, nil
244244
default:
245-
return "", fmt.Errorf("environment variable %q can only be set to %s", "DAPR_DEFAULT_IMAGE_REGISTRY", "DOCKERHUB")
245+
return "", fmt.Errorf("environment variable %q can only be set to %s", "DAPR_DEFAULT_IMAGE_REGISTRY", "GHCR")
246246
}
247247
}

0 commit comments

Comments
 (0)