Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

docs(development_guide/READEME.md): add notes on missing configurations #5132

Closed
wants to merge 1 commit into from

Conversation

lam-man
Copy link
Contributor

@lam-man lam-man commented Sep 15, 2022

Add missing configurations for local development setup

Signed-off-by: Wen Lin [email protected]

Description:
While following the development guide to set up my local environment for development, I found the following missing configurations and it is not easy to figure out. I think it will be helpful for others who want to contributing to OSM.

  • Set GO111MODULE=on. The default value is off and will cause the errors while download dependencies using go get -d ./.... Example error:
      /<user-path-to>/go/1.19.1/libexec/src/github.com/go-gorp/gorp/v3 (from $GOROOT)
      /<user-path-to>/go/src/github.com/go-gorp/gorp/v3 (from $GOPATH)```
    
  • Set export CTR_TAG=latest-main in .env file. Without the tag, commands like make docker-build and make build-osm-all will have the following error:
    server message: insufficient_scope: authorization failed
    

Testing done:
Tested locally. No other tests needed.

Affected area:

Functional Area
New Functionality [ ]
CI System [ ]
CLI Tool [ ]
Certificate Management [ ]
Control Plane [ ]
Demo [ ]
Documentation [X ]
Egress [ ]
Ingress [ ]
Install [ ]
Networking [ ]
Observability [ ]
Performance [ ]
SMI Policy [ ]
Security [ ]
Sidecar Injection [ ]
Tests [ ]
Upgrade [ ]
Other [ ]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project? NO

    • Did you notify the maintainers and provide attribution? N/A
  2. Is this a breaking change? NO

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)? N/A

The OSM packages rely on many external Go libraries.

Take a peek at the `go.mod` file in the root of this repository to see all dependencies.
The OSM packages rely on many external Go libraries. Take a peek at the [go.mod](https://github.com/openservicemesh/osm/blob/main/go.mod) file in the root of this repository to see all dependencies.

Run `go get -d ./...` to download all required Go packages.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is your go version and are you running these commands in your GOPATH?

I thought running go get is no longer required with never versions of golang, but it might be becuase I don't use GOPATH:

go help gopath-get
The 'go get' command changes behavior depending on whether the
go command is running in module-aware mode or legacy GOPATH mode.

should this maybe updated to go mod tidy? which shouldn't require setting GO111MODULE.

I think since 1.17+ GO111MODULE=on is the default behavior. In my local env I don't have it set:

❯ go env
GO111MODULE=""

Copy link
Contributor Author

@lam-man lam-man Sep 17, 2022

Choose a reason for hiding this comment

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

My go version is : go version go1.19.1 darwin/amd64 . I think this is the latest version. Interestingly, GO111MODULE is off by default. I am not sure this is because I installed go through Homebrew.

Copy link
Contributor

Choose a reason for hiding this comment

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

is it off or empty? If it is off I would be surprised but could be something to do with the homebrew installation

(NOTE: these requirements are true for automatic demo deployment using the available demo scripts; [#1416](https://github.com/openservicemesh/osm/issues/1416) tracks an improvement to not strictly require these and use upstream images from official dockerhub registry if a user does not want/need changes on the code)
> Note: These requirements are true for automatic demo deployment using the available demo scripts; [#1416](https://github.com/openservicemesh/osm/issues/1416) tracks an improvement to not strictly require these and use upstream images from official dockerhub registry if a user does not want/need changes on the code

> Note: For local development, you need to set the `CTR_TAG` environment variable. Without `CTR_TAG`, commands like `make docker-build` and `make build-osm-all` will fail. Example: `export CTR_TAG=latest-main` .
Copy link
Contributor

Choose a reason for hiding this comment

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

this is surprising as well. The tag is defaulted in the make file:

osm/Makefile

Line 7 in 444ccbf

CTR_TAG ?= latest-main

did you modify any of the env files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't change the env file. I tested again with main branch again and you are CORRECT. I think the error might be resolved by another fix I did: log into docker in command line and log into DockerHub on my Docker desktop. I will change Note. The fix I did was from a docker issue.

@jsturtevant
Copy link
Contributor

@lam-man sorry you had so much trouble, If you run into issues you can always reach out in slack (info on how to join in https://github.com/openservicemesh/osm#community)

@lam-man
Copy link
Contributor Author

lam-man commented Sep 17, 2022

@lam-man sorry you had so much trouble, If you run into issues you can always reach out in slack (info on how to join in https://github.com/openservicemesh/osm#community)

Hi James. Not a problem at all. I am in the community Slack. I will try to ask setup questions in the slack channel next time. Thanks for your replies.

@lam-man
Copy link
Contributor Author

lam-man commented Sep 23, 2022

I think the notes I added didn't apply to all. I will close this PR. However, I still think a revisit of the development guide is needed. Thanks!

@lam-man lam-man closed this Sep 23, 2022
@jsturtevant
Copy link
Contributor

I think the notes I added didn't apply to all. I will close this PR. However, I still think a revisit of the development guide is needed. Thanks!

If you have suggestions would be happy to review

@lam-man
Copy link
Contributor Author

lam-man commented Sep 23, 2022

I think the notes I added didn't apply to all. I will close this PR. However, I still think a revisit of the development guide is needed. Thanks!

If you have suggestions would be happy to review

Thanks James. I will go over the document again. Those problems could be caused by the way I install my Golang.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants