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.

Sign up for GitHub

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

Jump to bottom

General code tidy #4

Merged
merged 15 commits into from
May 29, 2020
Merged

General code tidy #4

merged 15 commits into from
May 29, 2020

Conversation

mark-au
Copy link
Contributor

@mark-au mark-au commented May 20, 2020 •
edited
Loading

Non-functional changes:

  • OLCNE ships multiple modules when this repo uses the term 'module' to refer to the Kubernetes module. Rename references to Kubernetes module to allow easier future PRs that refer to new modules
  • Multiple shell scripts are delivered without a file extension, add one for readability
  • Improve formatting and content of user facing strings
  • Be consistent when using double quotes for variables

Functional changes that have no impact to outcome:

  • Close unnecessary firewall ports
  • Disable swap across reboots

Signed-off-by: Mark Cram mark.cram@oracle.com

mark-au added 10 commits May 20, 2020 14:31
@mark-au
Avoid use of shorthand module to when referring to the Kubernetes module
f909a08 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Give scripts an extension to indicate their usage
e02b97e 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Not necessary to open NodePorts
e126c3a 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Persistently disable swap
a2a1a0d 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Improve readability
0c394b2 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Continue rename of 'module' to 'Kubernetes module'
b15eea1 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Hotfix to e02b97e - Give scripts an extension to indicate their usage
d3f2599 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Continue rename of 'module' to 'Kubernetes module'
7fd34da 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Update user facing strings to include capitalisation
6c19487 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Be consistent when using double quotes for globbing
58879b7 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Copy link
Contributor Author

mark-au commented May 20, 2020

@hyder do you perform squash merges on PRs or would you rather I rebase this to join a few commits together? I was going to do the latter but realized it may be wasted if you squash on merge.

Djelibeybi
Djelibeybi requested changes May 21, 2020
View reviewed changes
Copy link

@Djelibeybi Djelibeybi left a comment

Choose a reason for hiding this comment

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

I've waved my editorial red pen across the draft. :)

modules/operator/scripts/configure_agent.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/configure_agent.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/configure_api.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/configure_api.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/configure_kata.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/install_helm.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/install_kubernetes_module.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/install_kubernetes_module.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/install_kubernetes_module.template.sh Outdated Show resolved Hide resolved
modules/operator/scripts/install_nginx.template.sh Outdated Show resolved Hide resolved
@mark-au
Improve readability of user facing messages
f7be445 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Allow expansion of variables in echo
7dd8b95 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@mark-au
Copy link
Contributor Author

mark-au commented May 21, 2020

@hyder has confirmed he will be squashing during merge so I won't rebase the commits on this branch; I was going to do this for readability.

@mark-au mark-au marked this pull request as ready for review May 21, 2020 04:50
@mark-au mark-au requested a review from Djelibeybi May 21, 2020 05:05
@mark-au
ssh-keyscan(1) matches the first hostname in a string, don't use quotes
eb24b87 
Signed-off-by: Mark Cram <mark.cram@oracle.com>
@hyder
Copy link
Contributor

hyder commented May 21, 2020

Let's split cosmetic and operational changes in future.

@mark-au
Copy link
Contributor Author

mark-au commented May 25, 2020

I noted when testing that the nginx-nginx-ingress-nginxdefaultbackend deployment does not come online but it does not come online in the master branch either so I see no issues.

hyder
hyder reviewed May 26, 2020
View reviewed changes
modules/master/locals.tf Outdated Show resolved Hide resolved
@hyder
Copy link
Contributor

hyder commented May 26, 2020

I noted when testing that the nginx-nginx-ingress-nginxdefaultbackend deployment does not come online but it does not come online in the master branch either so I see no issues.

#5

@hyder hyder merged commit 77968bd into oracle-terraform-modules:master May 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hyder hyder hyder left review comments

@Djelibeybi Djelibeybi Djelibeybi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mark-au @hyder @Djelibeybi