update secret names and doc for osa #1347

mjudeikis · 2018-09-03T14:26:29Z

cc: @openshift/sig-azure

0xmichalis · 2018-09-03T14:27:36Z

projects/azure/README.md

s/contains/contain/

Both of them contains same data.

They don't actually contain the same data. More precisely the format differs. We should probably call out that the env based secret is what we use for deploying clusters in Azure (so it's a drop-in config file for Openshift) whereas the file based approach is what ci-operator expects in order to source it in its environment.

Sorry, azure-purge is using the env based secret. Is it the same as azure.conf? If not disregard my previous comment.

One is just export FOO=BAR in the file, second one is --from-literal=foo=${BAR}. Same credentials, same variables just format.

jim-minter · 2018-09-03T20:20:04Z

can't we re-engineer to make cluster-secrets-azure-file go away instead?

0xmichalis · 2018-09-04T06:10:42Z

Unfortunately it's needed by the ci-operator. We could potentially have an init container in the ci-operator pod that reads the azure-purge secret and writes down the ci-operator format. We'd still need to keep the azure-purge secret in two different place (azure and ci namespace) until the developer productivity team creates the secret mirror controller.

…

On Mon, Sep 3, 2018, 22:20 Jim Minter ***@***.***> wrote: can't we re-engineer to make cluster-secrets-azure-file go away instead? — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#1347 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADuFf15FEjHoqs00vMjwFq_jTFBL7i7Bks5uXY71gaJpZM4WXtdl> .

mjudeikis · 2018-09-04T06:26:42Z

/hold

mjudeikis · 2018-09-04T06:32:07Z

@jim-minter
It would make local development super hard (its not easy now due how ci-operator is built and how we use it). Second we would be using it differently like everybody else, so understand our thinking would be harder without asking us directly

Current flow:

https://github.com/openshift/release/blob/master/ci-operator/jobs/openshift/openshift-azure/openshift-openshift-azure-postsubmits-release-3.11.yaml#L47-L51 secret is projected and mounted to :

mountPath: /usr/local/e2e-azure-secret
name: cluster-secrets-azure

Next folder is passed to ci-operator as parameter:

- --secret-dir=/usr/local/e2e-azure-secret

Parameter is being "projected" again in "CI-Operator way" to test namespace in the format. This part is CI operator generated and expects file based secrets Projected to one folder with all ConfigMaps, Secrets.

    - name: cluster-secrets-azure-file
      secret:
        secretName: e2e-azure-secret

Where the secret name is the folder name we passed into argument. And it comes available to our ci-namespace

Considering we are could move to env based secret here, but now we need to do local development.

ci-operator --config ci-operator/config/openshift/openshift-azure/master.json --namespace=namespace-name --git-ref=openshift/openshift-azure@master --template ci-operator/templates/cluster-launch-e2e-azure.yaml --secret-dir $(pwd)/cluster/test-deploy/azure/

Which involved passing directory as a secret.

  -secret-dir value
        One or more directories that should converted into secrets in the test namespace.

Considering we do all this and make this tiny miny secret same, we get all this burden of "doing things our way + local development"

We could do as you suggest few weeks ago and mount env as file secret directly and result would be:

/etc/azure/credentials $ ls -la
total 0
drwxrwsrwt    3 root     10001500       140 Sep  4 06:26 .
drwxr-xr-x    3 root     root            25 Sep  4 06:26 ..
drwxr-sr-x    2 root     10001500       100 Sep  4 06:26 ..2018_09_04_06_26_35.288494078
lrwxrwxrwx    1 root     root            31 Sep  4 06:26 ..data -> ..2018_09_04_06_26_35.288494078
lrwxrwxrwx    1 root     root            22 Sep  4 06:26 azure_client_id -> ..data/azure_client_id
lrwxrwxrwx    1 root     root            26 Sep  4 06:26 azure_client_secret -> ..data/azure_client_secret
lrwxrwxrwx    1 root     root            22 Sep  4 06:26 azure_tenant_id -> ..data/azure_tenant_id

We still need to do sourcing of individual files, instead of one and for local development I have to prepare myself 4 files, instead of one.

Answer is - yes its possible, and it might be right way from secret point of view, but it becomes burden we will need to maintain down the stream in a form of unnecessary complexity.

petr-muller · 2018-09-10T13:14:21Z

About the rebase request:

We have mass-renamed all ci-operator configuration files from .json to .yaml in #1336, because YAML is what ci-operator consumes now. All the references of these files needed to be changed too. JSON is a YAML subset so that the actual file content can stay like it is (we'll eventually mass-reformat them to the block style).

mjudeikis · 2018-09-10T13:18:43Z

@petr-muller This was fast :) will fix this accordingly

mjudeikis · 2018-09-11T16:45:30Z

ping @jim-minter

jim-minter · 2018-09-11T23:47:33Z

thanks for the explanation @mjudeikis - go ahead as you were :)

/hold cancel

mjudeikis · 2018-09-12T10:16:58Z

Post-merge these need to be executed:

oc get secret cluster-secrets-azure -o yaml --export  -n azure | sed s/cluster-secrets-azure/cluster-secrets-azure-env/ | oc apply -f -
oc get secret cluster-secrets-azure-temp -o yaml --export -n azure | sed s/cluster-secrets-azure-temp/cluster-secrets-azure-file/ | oc apply -n ci -f -

mjudeikis · 2018-09-12T10:17:50Z

ping @stevekuznetsov @Kargakis

0xmichalis · 2018-09-12T10:23:18Z

projects/azure/README.md

the same data but in a different format

which of course can be argued that it's not the same data :)

In can :) I will be happy to argue with anybody who is not you at any time when somebody comes and pick this as maintenance job :D

0xmichalis · 2018-09-12T10:25:45Z

New secrets created

mjudeikis · 2018-09-12T10:48:54Z

lets merge it after lunch as if I left some typos in renaming all stuff will go down :D

0xmichalis · 2018-09-12T12:53:12Z

/lgtm

openshift-ci-robot · 2018-09-12T12:53:44Z

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: kargakis, mjudeikis

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

OWNERS

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

openshift-ci-robot · 2018-09-12T12:54:29Z

@mjudeikis: Updated the job-config configmap using the following files:

key openshift-openshift-azure-postsubmits.yaml using file ci-operator/jobs/openshift/openshift-azure/openshift-openshift-azure-postsubmits.yaml
key openshift-openshift-azure-presubmits.yaml using file ci-operator/jobs/openshift/openshift-azure/openshift-openshift-azure-presubmits.yaml

Details

In response to this:

ptal: @Kargakis , @jim-minter

cc: @openshift/sig-azure

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2018-09-12T12:54:31Z

@mjudeikis: The following updates succeeded:

/usr/bin/make apply WHAT=projects/azure/azure-purge/cronjob.yaml


$ /usr/bin/make apply WHAT=projects/azure/azure-purge/cronjob.yaml
oc apply -f projects/azure/azure-purge/cronjob.yaml
cronjob "azure-purge" configured

Details

In response to this:

ptal: @Kargakis , @jim-minter

cc: @openshift/sig-azure

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added the sig/azure Categorizes item related to Azure jobs label Sep 3, 2018

openshift-ci-robot requested review from 0xmichalis and kwoodson September 3, 2018 14:26

openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 3, 2018

0xmichalis reviewed Sep 3, 2018

View reviewed changes

mjudeikis force-pushed the osa-docs branch from 2558881 to d33822c Compare September 3, 2018 15:53

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 4, 2018

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 8, 2018

mjudeikis force-pushed the osa-docs branch from d33822c to 4958cac Compare September 10, 2018 13:14

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 10, 2018

mjudeikis force-pushed the osa-docs branch from 4958cac to 844573c Compare September 10, 2018 13:16

mjudeikis force-pushed the osa-docs branch from 844573c to b22b5fc Compare September 10, 2018 13:19

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 11, 2018

mjudeikis force-pushed the osa-docs branch 2 times, most recently from daf839c to a09b3f9 Compare September 12, 2018 10:13

0xmichalis reviewed Sep 12, 2018

View reviewed changes

update secret names and doc for osa

1a95361

mjudeikis force-pushed the osa-docs branch from a09b3f9 to 1a95361 Compare September 12, 2018 12:51

openshift-ci-robot assigned 0xmichalis Sep 12, 2018

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 12, 2018

0xmichalis added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 12, 2018

openshift-merge-robot merged commit 1e98535 into openshift:master Sep 12, 2018

derekhiggins pushed a commit to derekhiggins/release that referenced this pull request Oct 24, 2023

Disable ansible clean up for CI instances (openshift#1347)

a12d34d

update secret names and doc for osa #1347

update secret names and doc for osa #1347

Uh oh!

Conversation

mjudeikis commented Sep 3, 2018

Uh oh!

0xmichalis Sep 3, 2018

Choose a reason for hiding this comment

Uh oh!

0xmichalis Sep 3, 2018

Choose a reason for hiding this comment

Uh oh!

0xmichalis Sep 3, 2018

Choose a reason for hiding this comment

Uh oh!

mjudeikis Sep 3, 2018

Choose a reason for hiding this comment

Uh oh!

jim-minter commented Sep 3, 2018

Uh oh!

0xmichalis commented Sep 4, 2018 via email

Uh oh!

mjudeikis commented Sep 4, 2018

Uh oh!

mjudeikis commented Sep 4, 2018

Uh oh!

petr-muller commented Sep 10, 2018

Uh oh!

mjudeikis commented Sep 10, 2018

Uh oh!

mjudeikis commented Sep 11, 2018

Uh oh!

jim-minter commented Sep 11, 2018

Uh oh!

mjudeikis commented Sep 12, 2018 • edited by 0xmichalis Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjudeikis commented Sep 12, 2018

Uh oh!

0xmichalis Sep 12, 2018

Choose a reason for hiding this comment

Uh oh!

mjudeikis Sep 12, 2018

Choose a reason for hiding this comment

Uh oh!

0xmichalis commented Sep 12, 2018

Uh oh!

mjudeikis commented Sep 12, 2018

Uh oh!

0xmichalis commented Sep 12, 2018

Uh oh!

openshift-ci-robot commented Sep 12, 2018

Uh oh!

openshift-ci-robot commented Sep 12, 2018

Uh oh!

openshift-ci-robot commented Sep 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mjudeikis commented Sep 12, 2018 •

edited by 0xmichalis

Loading