MGMT-1858 - subsystem should use dummy ignition image and not a real one #154

yevgeny-shnaidman · 2020-08-18T09:14:15Z

No description provided.

app-sre-bot · 2020-08-18T09:14:17Z

Can one of the admins verify this patch?

yevgeny-shnaidman · 2020-08-18T09:14:49Z

filanov · 2020-08-18T09:25:04Z

Makefile

@@ -78,14 +79,17 @@ generate-keys:
 ##################

 .PHONY: build
-build: lint unit-test build-minimal build-iso-generator generate-keys
+build: lint unit-test build-minimal build-iso-generator build-dummy-ignition generate-keys


it should not be part of build

filanov · 2020-08-18T09:26:26Z

Makefile

@@ -104,10 +108,13 @@ update-minimal: build-minimal
 	GIT_REVISION=${GIT_REVISION} docker build --network=host --build-arg GIT_REVISION \
 		-f Dockerfile.assisted-service . -t $(SERVICE)

-update-minikube: build
+update-minikube: build build-dummy-ignition-image


it should be in jenkins-deploy-for-subsystem as well

filanov · 2020-08-18T09:27:59Z

pkg/job/job.go

@@ -302,6 +303,10 @@ func (k *kubeJob) GenerateISO(ctx context.Context, cluster common.Cluster, jobNa
 func (k *kubeJob) createKubeconfigJob(cluster *common.Cluster, jobName string, cfg []byte, encodedDhcpFileContents string) *batch.Job {
 	id := cluster.ID
 	ignitionGeneratorImage := k.Config.IgnitionGenerator
+	var pullPolicy core.PullPolicy = "Always"
+	if k.Config.SubsystemRun {


why not making the policy as a flag?

Why not have the always flag?
Since (see my comment on the default name of the image) will never be in external repo, always should work, no?

@ronniel1 always flag means that kuberenetes will try to pull the image from external repo every time, that's why we need to set never on subsystem

but in this case there will not be an external repo

ronniel1

Must also change the Jenkinsfile for this to work in jenkins

ronniel1 · 2020-08-18T09:41:17Z

Makefile

@@ -78,14 +79,17 @@ generate-keys:
 ##################

 .PHONY: build
-build: lint unit-test build-minimal build-iso-generator generate-keys
+build: lint unit-test build-minimal build-iso-generator build-dummy-ignition generate-keys


ronniel1 · 2020-08-18T09:44:55Z

Makefile

@@ -20,6 +20,7 @@ endif # TARGET

 SERVICE := $(or ${SERVICE},quay.io/ocpmetal/assisted-service:latest)
 ISO_CREATION := $(or ${ISO_CREATION},quay.io/ocpmetal/assisted-iso-create:latest)
+DUMMY_IGNITION := $(or ${DUMMY_IGNITION},quay.io/ocpmetal/ignition-dummy:latest)


We should never push the dummy ignition to ocpmetal, so the default should be something local. for example minikube-local-registry/ignition-dummy-generator

ronniel1 · 2020-08-18T09:45:53Z

pkg/job/job.go

@@ -302,6 +303,10 @@ func (k *kubeJob) GenerateISO(ctx context.Context, cluster common.Cluster, jobNa
 func (k *kubeJob) createKubeconfigJob(cluster *common.Cluster, jobName string, cfg []byte, encodedDhcpFileContents string) *batch.Job {
 	id := cluster.ID
 	ignitionGeneratorImage := k.Config.IgnitionGenerator
+	var pullPolicy core.PullPolicy = "Always"
+	if k.Config.SubsystemRun {


Why not have the always flag?
Since (see my comment on the default name of the image) will never be in external repo, always should work, no?

filanov · 2020-08-18T10:01:12Z

@yevgeny-shnaidman @ronniel1 why not make it part of the ignition repo?
It's strange the assisted-service need to build a mock for a different service no?
maybe we can have this code and the build process in the ignition repo?

yevgeny-shnaidman · 2020-08-18T10:30:03Z

@filanov since assisted-service is using this image, then i think it is more logical that it implements the mock. In addition, if we move the mock to ignition repo, then you need to actually deliver it to remote repo

yevgeny-shnaidman · 2020-08-18T10:54:57Z

@ronniel1 "always" flag means that kuberenetes will try to access remote repository, without regards to whether image present locally or not

ronniel1 · 2020-08-18T10:48:39Z

pkg/job/job.go

@@ -302,6 +303,10 @@ func (k *kubeJob) GenerateISO(ctx context.Context, cluster common.Cluster, jobNa
 func (k *kubeJob) createKubeconfigJob(cluster *common.Cluster, jobName string, cfg []byte, encodedDhcpFileContents string) *batch.Job {
 	id := cluster.ID
 	ignitionGeneratorImage := k.Config.IgnitionGenerator
+	var pullPolicy core.PullPolicy = "Always"
+	if k.Config.SubsystemRun {


but in this case there will not be an external repo

ronniel1 · 2020-08-18T11:02:53Z

Makefile

-jenkins-deploy-for-subsystem:
-	export TEST_FLAGS=--subsystem-test && export ENABLE_AUTH="True" && $(MAKE) deploy-all
+jenkins-deploy-for-subsystem: build-dummy-ignition-image
+	export TEST_FLAGS=--subsystem-test && export ENABLE_AUTH="True" && export DUMMY_IGNITION=${DUMMY_IGNITION} && $(MAKE) deploy-all


export DUMMY_IGNITION=minikube-local-registry/ignition-dummy-generator:minikube-test

@ronniel1 kubernetes will still treat it as external and will try to connect

ronniel1 · 2020-08-18T11:04:30Z

/approve cancel

ronniel1 · 2020-08-18T13:54:46Z

/approve

openshift-ci-robot · 2020-08-18T13:54:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ronniel1, yevgeny-shnaidman

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [ronniel1,yevgeny-shnaidman]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ronniel1 · 2020-08-18T13:55:09Z

/lgtm

openshift/assisted-service#154 changed them from lower case to caps

openshift-ci-robot requested review from oshercc and RazRegev August 18, 2020 09:14

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 18, 2020

openshift-ci-robot assigned filanov and ronniel1 Aug 18, 2020

filanov reviewed Aug 18, 2020

View reviewed changes

ronniel1 reviewed Aug 18, 2020

View reviewed changes

filanov assigned YuviGold Aug 18, 2020

yevgeny-shnaidman added 2 commits August 18, 2020 14:00

MGMT-1858 - subsystem should use dummy ignition image and not a real one

dbec399

MGMT-1858 - subsystem should use dummy ignition image and not a real one

f529e07

ronniel1 reviewed Aug 18, 2020

View reviewed changes

MGMT-1858 - subsystem should use dummy ignition image and not a real one

d34ca91

yevgeny-shnaidman force-pushed the yevgeny/MGMT-1858 branch from 13a6975 to d34ca91 Compare August 18, 2020 11:29

ronniel1 approved these changes Aug 18, 2020

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 18, 2020

openshift-merge-robot merged commit 2ed1b5a into openshift:master Aug 18, 2020

carbonin added a commit to carbonin/assisted-ignition-generator that referenced this pull request Aug 18, 2020

Use new env var names for aws creds

2fb1bdc

openshift/assisted-service#154 changed them from lower case to caps

carbonin mentioned this pull request Aug 18, 2020

Use new env var names for aws creds openshift/assisted-ignition-generator#15

Merged

openshift-merge-robot pushed a commit to openshift/assisted-ignition-generator that referenced this pull request Aug 18, 2020

Use new env var names for aws creds (#15)

deb5257

openshift/assisted-service#154 changed them from lower case to caps

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-1858 - subsystem should use dummy ignition image and not a real one #154

MGMT-1858 - subsystem should use dummy ignition image and not a real one #154

yevgeny-shnaidman commented Aug 18, 2020

app-sre-bot commented Aug 18, 2020

yevgeny-shnaidman commented Aug 18, 2020

filanov Aug 18, 2020

ronniel1 Aug 18, 2020

yevgeny-shnaidman Aug 18, 2020

filanov Aug 18, 2020

yevgeny-shnaidman Aug 18, 2020

filanov Aug 18, 2020

ronniel1 Aug 18, 2020

yevgeny-shnaidman Aug 18, 2020

ronniel1 Aug 18, 2020

ronniel1 left a comment

ronniel1 Aug 18, 2020

ronniel1 Aug 18, 2020

yevgeny-shnaidman Aug 18, 2020

ronniel1 Aug 18, 2020

filanov commented Aug 18, 2020

yevgeny-shnaidman commented Aug 18, 2020

yevgeny-shnaidman commented Aug 18, 2020

ronniel1 Aug 18, 2020

ronniel1 Aug 18, 2020

yevgeny-shnaidman Aug 18, 2020

ronniel1 commented Aug 18, 2020

ronniel1 commented Aug 18, 2020

openshift-ci-robot commented Aug 18, 2020

ronniel1 commented Aug 18, 2020

MGMT-1858 - subsystem should use dummy ignition image and not a real one #154

MGMT-1858 - subsystem should use dummy ignition image and not a real one #154

Conversation

yevgeny-shnaidman commented Aug 18, 2020

app-sre-bot commented Aug 18, 2020

yevgeny-shnaidman commented Aug 18, 2020

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

ronniel1 left a 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

filanov commented Aug 18, 2020

yevgeny-shnaidman commented Aug 18, 2020

yevgeny-shnaidman commented Aug 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ronniel1 commented Aug 18, 2020

ronniel1 commented Aug 18, 2020

openshift-ci-robot commented Aug 18, 2020

ronniel1 commented Aug 18, 2020