[GitHub] [apisix-ingress-controller] membphis commented on a change in pull request #61: chore: enhance project structure

[GitBox <gi...@apache.org>]

membphis commented on a change in pull request #61:
URL: https://github.com/apache/apisix-ingress-controller/pull/61#discussion_r538162636



##########
File path: pkg/utils/utils.go
##########
@@ -0,0 +1,23 @@
+package utils
+
+import (
+	"fmt"
+	"os"
+	"strings"
+)
+
+// Dief renders the template string with provided args, outputs it to stderr and
+// exits the program with code 1.
+func Dief(template string, args ...interface{}) {
+	if !strings.HasSuffix(template, "\n") {
+		template += "\n"
+	}
+	fmt.Fprintf(os.Stderr, template, args...)
+	os.Exit(1)
+}
+
+// Die outputs the provided args to stderr and exits the program with code 1.
+func Die(args ...interface{}) {
+	fmt.Fprintln(os.Stderr, args...)
+	os.Exit(1)
+}

Review comment:
       pls check your source code editor
   
   ![image](https://user-images.githubusercontent.com/6814606/101463468-6cd9bd80-3978-11eb-8d8f-23796d3fa44a.png)
   

##########
File path: pkg/config/init.go
##########
@@ -1,105 +1,105 @@
-package conf
+package config
 
 import (
+	"encoding/json"
+	clientSet "github.com/gxthrj/apisix-ingress-types/pkg/client/clientset/versioned"
+	"io/ioutil"
+	"k8s.io/client-go/informers"
 	coreinformers "k8s.io/client-go/informers/core/v1"
+	"k8s.io/client-go/kubernetes"
 	restclient "k8s.io/client-go/rest"
-	clientSet "github.com/gxthrj/apisix-ingress-types/pkg/client/clientset/versioned"
-	seven "github.com/gxthrj/seven/conf"
 	"k8s.io/client-go/tools/clientcmd"
-	"k8s.io/client-go/kubernetes"
-	"k8s.io/client-go/informers"
 	"os"
-	"path/filepath"
-	"io/ioutil"
-	"fmt"
-	"github.com/tidwall/gjson"
 	"runtime"
 )
 
 var (
-	ENV      string
-	basePath string
-	ADMIN_URL = os.Getenv("APISIX_ADMIN_INTERNAL")
-	HOSTNAME = os.Getenv("HOSTNAME")
-	LOCAL_ADMIN_URL = ""
-	podInformer coreinformers.PodInformer
-	svcInformer coreinformers.ServiceInformer
-	nsInformer coreinformers.NamespaceInformer
+	_hostname    string
+	config       *restclient.Config
+
+	// Deprecate: will be removed in the near future without notifications.
+	SyslogServer string
+)
+
+func init() {
+	hostname, err := os.Hostname()
+	if err != nil {
+		panic(err)
+	}
+	_hostname = hostname
+}
+
+var (
+	ENV               string
+	basePath          string
+	ADMIN_URL         = os.Getenv("APISIX_ADMIN_INTERNAL")
+	HOSTNAME          = os.Getenv("HOSTNAME")
+	LOCAL_ADMIN_URL   = ""
+	podInformer       coreinformers.PodInformer
+	svcInformer       coreinformers.ServiceInformer
+	nsInformer        coreinformers.NamespaceInformer
 	EndpointsInformer coreinformers.EndpointsInformer
-	IsLeader = false
+	IsLeader          = false
 	//etcdClient client.Client
-	kubeClient kubernetes.Interface
+	kubeClient                kubernetes.Interface
 	CoreSharedInformerFactory informers.SharedInformerFactory
 )
+
 const PROD = "prod"
 const HBPROD = "hb-prod"
 const BETA = "beta"
 const DEV = "dev"
 const TEST = "test"
 const LOCAL = "local"
-const confPath = "/root/ingress-controller/conf.json"
 const AispeechUpstreamKey = "/apisix/customer/upstream/map"
 
-func setEnvironment() {
-	if env := os.Getenv("ENV"); env == "" {
-		ENV = LOCAL
-	} else {
-		ENV = env
-	}
-	_, basePath, _, _ = runtime.Caller(1)
-}
+// Config contains necessary config items for running apisix-ingress-controller.
+type Config struct {
+	Hostname string `json:"-"`
 
-func ConfPath() string {
-	if ENV == LOCAL {
-		return filepath.Join(filepath.Dir(basePath), "conf.json")
-	} else {
-		return confPath
-	}
+	SyslogServer string       `json:"syslog_server"`
+	Kubeconfig   string       `json:"kubeconfig"`
+	Etcd         EtcdConfig   `json:"etcd"`
+	APISIX       APISIXConfig `json:"apisix"`
 }
 
-type etcdConfig struct {
-	Addresses []string
+// EtcdConfig contains config items about etcd.
+type EtcdConfig struct {
+	Endpoints []string `json:"endpoints"`
 }
 
-var EtcdConfig etcdConfig
-var K8sAuth k8sAuth
-var Syslog syslog
-
-var config *restclient.Config
+// APISIXConfig contains config items about apisix.
+type APISIXConfig struct {
+	BaseURL string `json:"base_url"`
+}
 
-func init() {
-	// 获取当前环境
-	setEnvironment()
-	// 获取配置文件路径
-	filePath := ConfPath()
-	// 获取配置文件内容
-	if configurationContent, err := ioutil.ReadFile(filePath); err != nil {
-		panic(fmt.Sprintf("failed to read configuration file: %s", filePath))
-	} else {
-		configuration := gjson.ParseBytes(configurationContent)
-		// apisix baseUrl
-		apisixConf := configuration.Get("conf.apisix")
-		apisixBaseUrl := apisixConf.Get("base_url").String()
-		seven.SetBaseUrl(apisixBaseUrl)
-		// k8sAuth conf
-		k8sAuthConf := configuration.Get("conf.k8sAuth")
-		K8sAuth.file = k8sAuthConf.Get("file").String()
-		// syslog conf
-		syslogConf := configuration.Get("conf.syslog")
-		Syslog.Host = syslogConf.Get("host").String()
+// NewDefaultConfig creates a Config object filled by default value.
+func NewDefaultConfig() *Config {
+	return &Config{
+		Hostname: _hostname,
 	}
-	// init etcd client
-	//etcdClient = NewEtcdClient()
-	// init informer
-	InitInformer()
 }
 
-type k8sAuth struct {
-	file string
+// NewConfigFromFiles creates a Config object and fill it by values in configuration file.
+func NewConfigFromFile(configPath string) (*Config, error) {
+	data, err := ioutil.ReadFile(configPath)
+	if err != nil {
+		return nil, err

Review comment:
       I think we need to capture the error and write it to the error log file.
   
   we can do this in a new PR. please create a new issue about this.

##########
File path: Makefile
##########
@@ -0,0 +1,33 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+default: build

Review comment:
       the default is `help` is better




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org