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.

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

Fix/update acm asm #50

Merged
merged 25 commits into from
May 12, 2021
Merged

Conversation

dkassab
Copy link
Contributor

@dkassab dkassab commented Apr 30, 2021

Updating documentation and fixing formatting issues.

@daniel-cit
Copy link
Contributor

Hi @dkassab Thanks for the PR
there are two lint errors:

  • A missing newline
Step #1 - "lint-tests": Checking for missing newline at end of file
Step #1 - "lint-tests": Error: No newline at end of file ./6-anthos-install/README.md
  • A missing required argument
Step #1 - "lint-tests": terraform_validate ./app-foundation/4-projects/shared 
Step #1 - "lint-tests": 
Step #1 - "lint-tests": Error: Missing required argument
Step #1 - "lint-tests": 
Step #1 - "lint-tests":   on infra_pipeline.tf line 44, in module "infra_pipelines":
Step #1 - "lint-tests":   44: module "infra_pipelines" {
Step #1 - "lint-tests": 
Step #1 - "lint-tests": The argument "impersonate_service_account" is required, but no definition was
Step #1 - "lint-tests": found

The missing required argument was fixed in PR #40
It should be fixed if you rebase with the current version of main

Copy link
Contributor

@daniel-cit daniel-cit left a comment

Choose a reason for hiding this comment

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

  • There are some changes that need to be reverted,
    which are the ones related to gcr.io/bank-of-anthos,
    they are marked individually in the files.
    The text gcr.io/bank-of-anthos is replaced automatically in step 7.

  • It would help the user if all the variables that depende in the output of previous steps
    would be defined in the define-required-environment-variables
    section for example these variable cloud be move to the section:

export CICD_PROJECT_ID=YOUR_CICD_PROJECT_ID

and

export SQL_PROJECT_ID=YOUR_SQL_PROJECT_ID
export SQL_INSTANCE_NAME_WEST=YOUR_SQL_INSTANCE_NAME_WEST
export SQL_INSTANCE_NAME_EAST=YOUR_SQL_INSTANCE_NAME_EAST
  • We have three project IDs in the README.md for step 6 and
    it would help the user if they were consistentelly named in the environement variable
    and in the replace tokens inside the yaml files
    • the GKE project ID:
      • PROJECT_ID -> GKE_PROJECT_ID (env var)
      • PROJECT_ID -> GKE_PROJECT_ID (yaml)
    • the CI/CD project ID
      • PROJECT_ID -> CICD_PROJECT_ID (env var)
      • PROJECT_ID -> CICD_PROJECT_ID (yaml
    • the SQL project ID
      • SQL_PROJECT_ID -> SQL_PROJECT_ID (env var)(keep)

for example, this command at the end of the readme

gcloud iam service-accounts add-iam-policy-binding \
--role roles/iam.workloadIdentityUser \
--member "serviceAccount:$PROJECT_ID.svc.id.goog[transactions/transactions]" \
boa-gsa@$PROJECT_ID.iam.gserviceaccount.com

should be using

  • PROJECT_ID -> GKE_PROJECT_ID

Since the boa-gsa service account is create in the GKE project.

6-anthos-install/README.md Outdated Show resolved Hide resolved
6-anthos-install/acm-repos/accounts/contacts.yaml Outdated Show resolved Hide resolved
6-anthos-install/acm-repos/transactions/ledgerwriter.yaml Outdated Show resolved Hide resolved
6-anthos-install/acm-repos/transactions/balancereader.yaml Outdated Show resolved Hide resolved
6-anthos-install/acm-repos/frontend/loadgenerator.yaml Outdated Show resolved Hide resolved
6-anthos-install/acm-repos/frontend/frontend.yaml Outdated Show resolved Hide resolved
6-anthos-install/acm-repos/accounts/userservice.yaml Outdated Show resolved Hide resolved
@google-cla
Copy link

google-cla bot commented May 1, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@rutalreja-deloitte
Copy link
Contributor

@googlebot I consent.

@rutalreja-deloitte
Copy link
Contributor

rutalreja-deloitte commented May 1, 2021

Fixes in c599738

@rutalreja-deloitte
Copy link
Contributor

rutalreja-deloitte commented May 1, 2021

@dkassab and @daniel-cit
Noticed missing instructions for

P.S Also a lot of files in the 6-anthos-install/acm-repos/root-config-repo/namespaces/boa/ repo folders are entirely commented out, maybe some instructions in the README are required to inform user what role the files play and at what stage

@dkassab
Copy link
Contributor Author

dkassab commented May 3, 2021

@dkassab and @daniel-cit
Noticed missing instructions for

P.S Also a lot of files in the 6-anthos-install/acm-repos/root-config-repo/namespaces/boa/ repo folders are entirely commented out, maybe some instructions in the README are required to inform user what role the files play and at what stage

  • I fixed acm root-sync.yaml file.
  • cloud-sql-admin-secret is not needed anymore, I removed it.
  • This secret is not needed for this phase, but will need it once we implement secret manager and CSI secret driver.

@rutalreja-deloitte
Copy link
Contributor

Fixes #51

  • Exposed bastion_members in 5-infra/env/main.tf and added supporting instruction in 6-README
  • Added missing Readme instruction for acm-repo-sync
  • Commented out app-secret-front

@rutalreja-deloitte
Copy link
Contributor

LGTM
@daniel-cit and @bharathkkb please review when you get a chance

Copy link
Contributor

@daniel-cit daniel-cit left a comment

Choose a reason for hiding this comment

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

This version has many improvements but there are still a few issues to fix

1. Creates a secret that grants access to the Kube API Server for cluster 1
```console
```
./istioctl x create-remote-secret \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
./istioctl x create-remote-secret \
istioctl x create-remote-secret \

Copy link
Member

Choose a reason for hiding this comment

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

It maybe valuable to keep ./. If the user has an older version of istioctl in PATH then it may error out with these configs

1. In a similar manner, create a secret that grants access to the Kube API Server for cluster 2
```console
```
./istioctl x create-remote-secret \
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
./istioctl x create-remote-secret \
istioctl x create-remote-secret \

6-anthos-install/README.md Outdated Show resolved Hide resolved
-C "GIT_REPO_USERNAME"
-N ''

Don't forget to upload the public key "~/.ssh/id_rsa.pub" to your repository. For cloud source repository, see [this link](https://cloud.google.com/source-repositories/docs/authentication)
Copy link
Contributor

Choose a reason for hiding this comment

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

is this repo in the CICD_PROJECT_ID ?

#### root config repo
This repository is the root repository that host cluster-scoped and namespace-scoped configs for the bank of anthos application, such as resource policies, network polices and security policies.
1. Clone the `root-config-repo` that was created through the infrastructure pipeline
```
gcloud source repos clone root-config-repo --project ${CICD_PROJECT_ID}
Copy link
Contributor

Choose a reason for hiding this comment

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

I got:

gcloud source repos clone root-config-repo --project ${CICD_PROJECT_ID}
WARNING:          You are using a Google-hosted repository with a
         git version 1.8.3.1
which is older than 2.0.1. If you upgrade
         to 2.0.1 or later, gcloud can handle authentication to
         this repository. Otherwise, to authenticate, use your Google
         account and the password found by running the following command.
          $ gcloud auth print-access-token

and I had to use as user boa-gce-bastion-d-sa@prj-bu1-d-boa-gke-04d8.iam.gserviceaccount.com
and the password generated by gcloud auth print-access-token

# On Cluster 2
kubectl create secret generic git-creds --namespace=transactions --context ${CTX_2} --from-file=ssh="${HOME}/.ssh/id_rsa"

kubectl create secret generic git-creds --namespace=accounts --context ${CTX_2} --from-file=ssh="${HOME}/.ssh/id_rsa"
Copy link
Contributor

Choose a reason for hiding this comment

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

failed with

error: failed to create secret namespaces "accounts" not found

It works after fixing

kubectl apply --context=${CTX_2} -f ${HOME}/terraform-example-foundation-app/6-anthos-install/acm-configs/config-management-west.yaml


kubectl create secret generic git-creds --namespace=accounts --context ${CTX_2} --from-file=ssh="${HOME}/.ssh/id_rsa"

kubectl create secret generic git-creds --namespace=frontend --context ${CTX_2} --from-file=ssh="${HOME}/.ssh/id_rsa"
Copy link
Contributor

Choose a reason for hiding this comment

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

failed with

error: failed to create secret namespaces "frontend" not found

It works after fixing

kubectl apply --context=${CTX_2} -f ${HOME}/terraform-example-foundation-app/6-anthos-install/acm-configs/config-management-west.yaml

6-anthos-install/README.md Outdated Show resolved Hide resolved

1. Run script to populate database ledger
```
kubectl apply -n transactions --context ${CTX_1} -f ${HOME}/terraform-example-foundation-app/6-anthos-install/db-scripts/populate-ledger-db.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

The file ${HOME}/terraform-example-foundation-app/6-anthos-install/db-scripts/populate-ledger-db.yaml does not exist


kubectl apply --context=${CTX_2} -f ${HOME}/terraform-example-foundation-app/acm-config/root-sync.yaml
kubectl apply -n transactions --context ${CTX_2} -f ${HOME}/terraform-example-foundation-app/6-anthos-install/db-scripts/populate-ledger-db.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

The file ${HOME}/terraform-example-foundation-app/6-anthos-install/db-scripts/populate-ledger-db.yaml does not exist

@daniel-cit
Copy link
Contributor

the populate script is in the Bank of Anthos repo

cd ${HOME}

git clone https://github.com/GoogleCloudPlatform/bank-of-anthos.git

kubectl apply -n transactions --context ${CTX_1} -f ${HOME}/bank-of-anthos/extras/cloudsql/populate-jobs/populate-ledger-db.yaml

kubectl apply -n transactions --context ${CTX_2} -f ${HOME}/bank-of-anthos/extras/cloudsql/populate-jobs/populate-ledger-db.yaml

Co-authored-by: Daniel Andrade <dandrade@ciandt.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants